Repository navigation
Conversation
codeant-ai-for-open-source
Bot
added
the
size:L
Sequence DiagramThis PR updates BigQuery SQL compilation so text filter values with apostrophes are always rendered as standard single quoted SQL literals. The flow highlights the new dialect patch and how query compilation now produces BigQuery safe filter SQL. sequenceDiagram
participant Superset
participant BigQueryEngineSpec
participant BigQueryDialect
participant SafeStringType
participant BigQuery
Superset->>BigQueryEngineSpec: Load BigQuery engine spec
BigQueryEngineSpec->>BigQueryDialect: Patch string literal handling
Superset->>BigQueryDialect: Compile query with literal binds
BigQueryDialect->>SafeStringType: Process text filter value
SafeStringType-->>BigQueryDialect: Return escaped single quoted literal
BigQueryDialect->>BigQuery: Execute query with valid filter literal
Generated by CodeAnt AI |
![bito-code-review[bot]](https://avatars.githubusercontent.com/in/1061978?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Agent Run #2b0fe4
Actionable Suggestions - 1
-
superset/db_engine_specs/bigquery.py - 1
- Incorrect % escaping in string literals · Line 100-100
Review Details
-
Files reviewed - 2 · Commit Range:
470b90a..470b90a- superset/db_engine_specs/bigquery.py
- tests/unit_tests/db_engine_specs/test_bigquery.py
-
Files skipped - 0
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
AI Code Review powered by
There was a problem hiding this comment.
Pull request overview
This PR fixes BigQuery query compilation failures when dashboard filter string values contain apostrophes by ensuring string literals are always rendered using standard SQL single-quote escaping under literal_binds=True.
Changes:
- Add a BigQuery dialect monkeypatch that overrides string literal rendering to always use single-quoted SQL literals with doubled internal quotes.
- Add unit tests intended to validate correct compilation for
=andIN (...)filters with and without apostrophes.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
superset/db_engine_specs/bigquery.py |
Monkeypatches sqlalchemy-bigquery string literal rendering via a TypeDecorator and custom literal processor. |
tests/unit_tests/db_engine_specs/test_bigquery.py |
Adds tests for compiled SQL string literals containing apostrophes and for IN-clause escaping. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #38835 +/- ##
==========================================
- Coverage 64.36% 64.36% -0.01%
==========================================
Files 2653 2653
Lines 144869 144892 +23
Branches 33424 33425 +1
==========================================
+ Hits 93247 93256 +9
- Misses 49951 49963 +12
- Partials 1671 1673 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Code Review Agent Run #40e3caActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
Code Review Agent Run #172fb3Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
|
@Krishnachaitanyakc I think this produces the wrong escape for BigQuery. The query in #35857 that fails is |
|
@Krishnachaitanyakc I think this produces the wrong escape for BigQuery. The query in #35857 that fails is |
|
@rusackas Thanks for catching this. I've pushed a fix that switches _process_string_literal to use ' instead of '', escapes existing backslashes first, and removes the unnecessary % → %% replacement. |
… quoting
The sqlalchemy-bigquery dialect uses Python's repr() to render string
literals when literal_binds=True. repr() switches to double-quote
delimiters when the string contains an apostrophe (e.g. repr("O'Brien")
produces "O'Brien"). In BigQuery SQL, double-quoted tokens are
identifiers, not string literals, so any filter containing an apostrophe
causes a syntax error.
This patch monkey-patches the BigQuery dialect's colspecs to use a custom
string type whose literal_processor always produces single-quoted literals
with properly doubled internal quotes (standard SQL escaping).
Fixes apache#35857
Add direct tests for _process_string_literal, non-BigQuery dialect fallback path in BigQuerySafeString.literal_processor, and monkeypatch verification to address missing coverage lines. Apply ruff formatting fix to bigquery.py.
Remove unused `type: ignore` comments from BigQuerySafeString class definition and colspecs assignment. Replace bare `assert BigQueryEngineSpec` with `assert BigQueryEngineSpec is not None` to satisfy mypy's truthy-function check.
Add tests for BigQuerySafeString.literal_processor bigquery branch and the ImportError fallback path to close Codecov patch coverage gaps.
BigQuery does not support doubled single-quote escaping ('O''Brien').
It parses this as two concatenated string literals without whitespace,
causing: 'concatenated string literals must be separated by whitespace'.
BigQuery requires backslash escaping ('O\'Brien') per its lexical rules.
Changes:
- _process_string_literal now uses backslash escaping (\' instead of '')
- Escape existing backslashes first (\ -> \\) before apostrophes
- Remove unnecessary percent escaping (BigQuery does not require it)
- Update all unit tests to assert backslash-escaped output
- Add negative assertions rejecting doubled-quote output
- Add edge-case coverage: backslash-in-value, percent passthrough
Follows the same pattern as the Databricks dialect fix
(ParamEscaper.escape_string in databricks.py).
8ca42b8 to
af8b50d
Compare
Code Review Agent Run #97e331Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
|
Fresh codex review for ya: This fixes the apostrophe case, but I think the new literal processor regresses other string values that
escaped = value.replace("\\", "\\\\").replace("'", "\\'")
return f"'{escaped}'"For a value containing an actual newline, e.g. 'foo
bar'BigQuery rejects that form. Its lexical docs say quoted strings cannot contain newlines, and newlines need to be represented with escape sequences such as Could we preserve escaping for control characters ( |
The previous backslash-escape fix only handled apostrophes and backslashes, so values containing real newlines, carriage returns, tabs, or other control characters were emitted as literal bytes inside a single-quoted BigQuery literal. BigQuery rejects that with "quoted strings cannot contain newlines" per its lexical rules. This change introduces an explicit escape table keyed off BigQuery's documented escape sequences (\n, \r, \t, \b, \f, \v, \a, \\, \') and falls back to \\xhh for any remaining C0 control character or DEL. \? \" and \` are intentionally omitted because they do not require escaping inside a single-quoted literal, and \0 is intentionally absent because BigQuery requires exactly three octal digits — the null byte falls through to the \\xhh fallback and is emitted as \\x00. Tests now cover: - exact-equality assertions for every named escape and several \\xhh fallback cases (null, 0x01, ESC, DEL) - literal backslash-then-n to confirm it does not collapse to a newline - double-quote pass-through - a negative assertion that no literal control character leaks into the output for any of the affected characters - an end-to-end SQLAlchemy compile-through-dialect test for a filter value containing an embedded newline
|
@rusackas Got it. Made changes with explicit escape table for BigQuery's named escapes (\n, \r, \t, \b, \f, \v, \a, \, ') with a \xhh fallback for any other C0/DEL. Please review |
Code Review Agent Run #d32113Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
rusackas
left a comment
rusackas
left a comment
There was a problem hiding this comment.
Thanks @Krishnachaitanyakc, LGTM! Backslash escaping is the right call (the doubled-quote version was the original bug), and the control-char escape table is a nice touch.
Heads up, I fixed up the squash commit message on merge, since the PR description still describes the old 'O''Brien' approach we abandoned. Merging now... thanks for sticking through the review rounds :)
rusackas
changed the title
…QL engine Adds testcontainers coverage for superset/db_engine_specs/bigquery.py's _monkeypatch_bigquery_string_literal, using the new BigQueryContainer (testcontainers/testcontainers-python#1121) to run apostrophe, percent-sign, and combined-value queries against a real GoogleSQL emulator rather than reasoning about the sqlalchemy-bigquery dialect and BigQuery DBAPI paramstyle handling from source alone. Also pins down, with a direct reproduction, why the doubled-single-quote escape #38835 replaced doesn't work on BigQuery. Co-Authored-By: Evan Rusackas <evan@preset.io> Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…QL engine Adds testcontainers coverage for superset/db_engine_specs/bigquery.py's _monkeypatch_bigquery_string_literal, using the new BigQueryContainer (testcontainers/testcontainers-python#1121) to run apostrophe, percent-sign, and combined-value queries against a real GoogleSQL emulator rather than reasoning about the sqlalchemy-bigquery dialect and BigQuery DBAPI paramstyle handling from source alone. Also pins down, with a direct reproduction, why the doubled-single-quote escape #38835 replaced doesn't work on BigQuery. Co-Authored-By: Evan Rusackas <evan@preset.io> Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…QL engine Adds testcontainers coverage for superset/db_engine_specs/bigquery.py's _monkeypatch_bigquery_string_literal, using the new BigQueryContainer (testcontainers/testcontainers-python#1121) to run apostrophe, percent-sign, and combined-value queries against a real GoogleSQL emulator rather than reasoning about the sqlalchemy-bigquery dialect and BigQuery DBAPI paramstyle handling from source alone. Also pins down, with a direct reproduction, why the doubled-single-quote escape #38835 replaced doesn't work on BigQuery. Co-Authored-By: Evan Rusackas <evan@preset.io> Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
User description
SUMMARY
Fixes #35857
BigQuery errors when dashboard filters on text columns contain apostrophes (e.g.
O'Brien,Fernando's).Root cause: The
sqlalchemy-bigquerydialect'sprocess_string_literalfunction uses Python'srepr()to render string literals whenliteral_binds=Trueis used during query compilation. When the string contains an apostrophe,repr()wraps the value in double quotes (e.g.repr("O'Brien")->"O'Brien"). In BigQuery SQL, double-quoted tokens are identifiers (like column or table names), not string literals, so the query fails with a syntax error.Fix: Monkey-patch the BigQuery dialect's
colspecsto use a customTypeDecoratorwhoseliteral_processoralways produces single-quoted literals with properly doubled internal quotes ('O''Brien'), which is the standard SQL escaping convention that BigQuery expects. This approach follows the same pattern used for the Databricks engine spec (superset/db_engine_specs/databricks.py).BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before: Filter with
Fernando'sgenerates SQLWHERE name = "Fernando's"(double-quoted identifier, causes BigQuery syntax error)After: Filter with
Fernando'sgenerates SQLWHERE name = 'Fernando''s'(properly escaped single-quoted literal)TESTING INSTRUCTIONS
O'Brien,Fernando's)Unit tests are included:
test_string_literal_with_apostrophe- verifies apostrophe escapingtest_string_literal_without_apostrophe- verifies normal strings unaffectedtest_string_literal_in_filter_with_apostrophe- verifies IN clause escapingADDITIONAL INFORMATION
CodeAnt-AI Description
Escape BigQuery filter values that contain apostrophes
What Changed
O'Briennow run in BigQuery instead of failing with a syntax errorImpact
✅ Fewer BigQuery filter errors✅ Clearer text filtering in dashboards✅ Reliable filters for names with apostrophes💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.