Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delete old name resolution, rewrite prepared statements #1927

Closed
wants to merge 15 commits into from

Conversation

max-hoffman
Copy link
Contributor

@max-hoffman max-hoffman commented Aug 7, 2023

  • Swap name resolution, fill gaps (many error tests with different names and other small tweaks).
  • Rewrite prepared statements to cache a sqlparser.ParsedQuery, whose GenerateQuery method converts a map of bindvars to a new query string. Caching the output of name resolution failed for many ORDER BY 1 clauses and compound expressions like SUM(x)+1. I don't think the better strategy is too far away, but better to not overcomplicate this right now.
  • I stripped many of the auxiliary prepared tests, keeping the core ones. The new prepare strategy is pretty simple, but I can add the other prepared tests back in if we want. I would probably structure them all in a different way.
  • The old parse.go tests are kept but disabled right now. I want to convert them to use the new name resolution, but the only thing I would be keeping are the query strings.
  • Started to delete some of the old code. More to do.

Working on Dolt bump. Some parse.go helper methods are used in strange ways, and still need to get tests working.

@max-hoffman max-hoffman marked this pull request as ready for review August 8, 2023 14:32
@max-hoffman max-hoffman requested a review from zachmu August 8, 2023 14:47
@Hydrocharged Hydrocharged deleted the max/delete-old-resolution branch February 7, 2024 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants