Skip to content

fix(bigquery): split native SQL scripts without borrowed grammar - #2306

Open
HandSonic wants to merge 5 commits into
OtterMind:mainfrom
HandSonic:fix/review-bigquery
Open

HandSonic wants to merge 5 commits into
OtterMind:mainfrom
HandSonic:fix/review-bigquery

Conversation

@HandSonic

@HandSonic HandSonic commented Jul 28, 2026 •

Copy link
Copy Markdown
Contributor

Problem

BigQuery reused the PostgreSQL parser for script splitting. PostgreSQL lexical rules do not understand BigQuery backtick identifiers, triple-quoted strings, or procedural blocks, so valid scripts could be split at semicolons that belong inside a single statement.

Replacing the parser with a borrowed MySQL grammar would also be incorrect because it rejects native BigQuery scripting syntax.

Fix

  • Keep the existing PostgreSQL-backed analysis and completion behavior.
  • Override only the in-memory and streaming split paths with a BigQuery lexical scanner.
  • Preserve semicolons inside quoted text, backticks, comments, nested brackets, BEGIN/IF/CASE/LOOP/WHILE/FOR/REPEAT blocks, and labeled blocks.
  • Keep reserved words in paths, parameters, table source lists, and DDL/DML targets out of script-block tracking.
  • Keep BEGIN TRANSACTION as a top-level statement.
  • Preserve UTF-8 text and streamed-file progress accounting.

Verification

  • BigQuery module: 29 tests, 0 failures/errors/skips on JDK 21 (BigQueryParserTest: 26 focused splitter tests).
  • Adversarial fuzz coverage for nested/no-whitespace CASE expressions, END suffixes, labels, comments, quoted sources, table lists, and native DDL/DML/LOAD targets.
  • git diff --check passed.

Fixes #2556

@openai0229

Copy link
Copy Markdown
Member

Blocking: this replaces the PostgreSQL parser with the MySQL grammar and visitors, not a BigQuery grammar. The test explicitly expects valid BigQuery MERGE syntax to fail, so the implementation treats a core native statement as invalid while claiming a BigQuery parser. Please open a standalone Bug issue using the Bug template, link the PR with Fixes #N, and either support the required BigQuery syntax or narrow the change to a tokenizer or splitter contract with valid native-syntax tests.

@openai0229 openai0229 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes based on the blocking maintainer review comment above. Please address that comment, link the required standalone Bug issue(s), and update the branch against current main before requesting re-review.

@openai0229 openai0229 changed the title fix(bigquery): parse BigQuery SQL with a backtick-aware grammar instead of the PostgreSQL parser fix(bigquery): split native SQL scripts without borrowed grammar Aug 4, 2026
@HandSonic
HandSonic force-pushed the fix/review-bigquery branch from d128e07 to ba5be5f Compare August 11, 2026 02:02
@HandSonic

Copy link
Copy Markdown
Contributor Author

Addressed the blocking review: the branch is rebased onto current main, the borrowed MySQL grammar/dialect is removed, and PostgreSQL-backed analysis remains unchanged. Only BigQuery script splitting is overridden with native lexical handling. Added the standalone bug link (Fixes #2556) and expanded focused coverage to 26 passing tests, including the adversarial path/CASE/END boundaries.

@HandSonic
HandSonic force-pushed the fix/review-bigquery branch from ba5be5f to 091d81d Compare August 11, 2026 02:36
@HandSonic

Copy link
Copy Markdown
Contributor Author

The first refreshed backend run exposed one stale grammar-coverage test left by the rejected MySQL-parser iteration. I restored that test to the current main PostgreSQL-analysis baseline and ran the full BigQuery module: 29/29 passed. Commit 091d81dbb is pushed and CI has restarted.

@HandSonic

Copy link
Copy Markdown
Contributor Author

@openai0229 The requested changes are now addressed and the refreshed CI is fully green, including backend tests and Java/JavaScript CodeQL. Please re-review when available.

@HandSonic
HandSonic force-pushed the fix/review-bigquery branch from 091d81d to 0c083d8 Compare August 11, 2026 13:33
@HandSonic

Copy link
Copy Markdown
Contributor Author

@openai0229 Rebase complete at 0c083d806 on current main@2442df98d; range-diff is equivalent and every refreshed check is green, including Java CodeQL. The PR is now CLEAN/MERGEABLE. Please re-review the addressed change request.

@openai0229 openai0229 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The earlier parser-design blocker is addressed: the current head keeps PostgreSQL-backed analysis and overrides only BigQuery script splitting, with native syntax coverage. One stale dependency from the rejected MySQL-parser iteration still blocks merge, though: line 23 depends on , while now extends again.\n\nPlease restore the direct dependency and rebase this old branch onto current , then rerun the BigQuery module tests and backend package. The reactor happens to compile today through transitive reactor dependencies, but the module POM no longer declares the class it actually uses.

@openai0229
openai0229 dismissed their stale review August 20, 2026 03:45

Superseded by a corrected review because shell quoting corrupted the submitted text.

@openai0229 openai0229 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The earlier parser-design blocker is addressed: the current head keeps PostgreSQL-backed analysis and overrides only BigQuery script splitting, with native syntax coverage. One stale dependency from the rejected MySQL-parser iteration still blocks merge, though: chat2db-community-bigquery/pom.xml line 23 depends on chat2db-community-mysql, while BigQueryParser now extends PgsqlSqlParser again.

Please restore the direct chat2db-community-postgresql dependency and rebase this old branch onto current main, then rerun the BigQuery module tests and backend package. The reactor happens to compile today through transitive reactor dependencies, but the module POM no longer declares the class it actually uses.

@HandSonic
HandSonic force-pushed the fix/review-bigquery branch from 0c083d8 to 1fbd82f Compare August 22, 2026 15:25
@HandSonic
HandSonic requested a review from Aias00 as a code owner August 22, 2026 15:25
Align existing OtterMind/Chat2DB PR OtterMind#2306 while preserving its focused fix.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Review

Development

Successfully merging this pull request may close these issues.

BigQuery script splitting breaks native quoted and procedural statements

2 participants