Conversation
There was a problem hiding this comment.
Code Review
This pull request enhances SQL parsing in PGAdapter by supporting queries wrapped in parentheses, handling 'materialized' and 'not materialized' CTE hints, and falling back to 'SELECT' when Spanner identifies a statement as a query but the simple parser cannot determine the command tag. The review feedback suggests simplifying the keyword-eating logic for CTE materialization hints and changing the visibility of skipOpeningParentheses() to private for better encapsulation.
Queries enclosed in parentheses (such as `(SELECT 1)`) had their command tag evaluated as an empty string by `SimpleParser.parseCommand(sql)`. This caused PGAdapter to treat the query as empty and send an `EmptyQueryResponse` to the client, returning zero rows. This change: - Updates `SimpleParser.parseCommand` to skip leading opening parentheses before checking for `WITH` clauses and before reading the command keyword. - Updates `SimpleParser.isCommand` to recognize parenthesized statements. - Handles `[NOT] MATERIALIZED` clauses when skipping CTE definitions. - Adds a defensive fallback in `IntermediateStatement` to use `"SELECT"` if `SimpleParser` yields an empty command tag for a statement Spanner classified as a query.
9ec1ec7 to
4531ec6
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request enhances the SQL parsing capabilities of PGAdapter to correctly handle queries starting with parentheses (e.g., (SELECT 1)) and CTEs using recursive, materialized, or not materialized keywords. It updates SimpleParser to skip opening parentheses when identifying command tags and improves IntermediateStatement to fall back to a SELECT command tag if the parser cannot determine it but Spanner's parser identifies the statement as a query. Comprehensive unit and mock server tests have been added to verify these changes. No review comments were provided, so there is no additional feedback.
Queries enclosed in parentheses (such as
(SELECT 1)) had their command tag evaluated as an empty string bySimpleParser.parseCommand(sql). This caused PGAdapter to treat the query as empty and send anEmptyQueryResponseto the client, returning zero rows.This change:
SimpleParser.parseCommandto skip leading opening parentheses before checking forWITHclauses and before reading the command keyword.SimpleParser.isCommandto recognize parenthesized statements.[NOT] MATERIALIZEDclauses when skipping CTE definitions.IntermediateStatementto use"SELECT"ifSimpleParseryields an empty command tag for a statement Spanner classified as a query.