Skip to content

fix: support queries starting with parentheses - #4973

Open
olavloite wants to merge 1 commit into
postgresql-dialectfrom
support-queries-starting-with-parentheses
Open

olavloite wants to merge 1 commit into
postgresql-dialectfrom
support-queries-starting-with-parentheses

Conversation

@olavloite

Copy link
Copy Markdown
Collaborator

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.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

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 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.

Comment thread src/main/java/com/google/cloud/spanner/pgadapter/statements/SimpleParser.java Outdated
Comment thread src/main/java/com/google/cloud/spanner/pgadapter/statements/SimpleParser.java Outdated
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.
@olavloite
olavloite force-pushed the support-queries-starting-with-parentheses branch from 9ec1ec7 to 4531ec6 Compare September 29, 2026 14:03
@olavloite

Copy link
Copy Markdown
Collaborator Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

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 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.

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.

1 participant