-
Notifications
You must be signed in to change notification settings - Fork 27
chore: add functions for parsing statement hint values #598
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
base: main
Are you sure you want to change the base?
Conversation
Adds functions for parsing statement hint values from SQL strings. These functions are currently
not used by the driver, will eventually be used to allow SQL strings to contain hints that sets
connection variables only for the duration of the execution of a statement. That is, the following
will eventually be supported:
```sql
@{ statement_tag='my_tag', statement_timeout='100ms' } select * from my_table where key=@key
```
The given statement_tag and statement_timeout will only be applied to the current statement.
|
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces functionality to parse statement hints for both GoogleSQL and PostgreSQL dialects, which will be used to set connection variables for a single statement. The changes include refactoring the SQL parser to handle whitespace options, updating hint skipping logic to be dialect-aware, and adding new functions to extract key-value pairs from these hints. The test suite has been expanded accordingly.
My review identifies a correctness issue where empty hints (e.g., @{}) are not parsed correctly. I've also suggested a couple of maintainability improvements: one to simplify prefix checking in the parser and another to refactor duplicated test logic for better clarity and easier maintenance.
|
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces functionality for parsing statement hints from SQL strings for both GoogleSQL and PostgreSQL dialects. The changes are well-structured, with new functions for hint extraction and key-value pair parsing. The refactoring to centralize whitespace handling is a good improvement. The code is accompanied by a comprehensive set of tests, which is great to see. I have a couple of suggestions to further improve code clarity and efficiency.
d311b82 to
d6f0995
Compare
Adds functions for parsing statement hint values from SQL strings. These functions are currently not used by the driver, will eventually be used to allow SQL strings to contain hints that sets connection variables only for the duration of the execution of a statement. That is, the following will eventually be supported:
@{ statement_tag='my_tag', statement_timeout='100ms' } select * from my_table where key=@keyThe given statement_tag and statement_timeout will only be applied to the current statement.