codeant-ai-for-open-source[bot] commented on code in PR #40455:
URL: https://github.com/apache/superset/pull/40455#discussion_r3308751750
##########
superset/db_engine_specs/bigquery.py:
##########
@@ -39,6 +39,22 @@
from sqlalchemy.engine.url import URL
from sqlalchemy.sql import column as sql_column, select, sqltypes
from sqlalchemy.sql.expression import table as sql_table
+from sqlalchemy.ext.compiler import compiles
+from sqlalchemy.sql.elements import BindParameter
+
+
+@compiles(BindParameter, "bigquery")
+def compile_bind_param_bigquery(element: BindParameter, compiler: Any, **kw:
Any) -> str:
+ """
+ BigQuery does not support standard SQL '' escaping natively for adjacent
string literals,
+ which causes 400 POST syntax errors on filters with apostrophes.
+ We override the bind parameter compilation to use \\' instead.
+ """
+ if kw.get("literal_binds") and isinstance(element.value, str):
+ val = element.value.replace("'", "\\'")
+ return f"'{val}'"
Review Comment:
**Suggestion:** The custom literal compiler only escapes single quotes, but
it leaves existing backslashes untouched before wrapping the value in quotes.
For inputs containing backslashes (especially values ending with `\`), this can
produce invalid BigQuery SQL like an escaped closing quote, causing query
failures and potentially corrupting literal boundaries. Escape backslashes
before apostrophes (or use the dialect literal processor and then transform
quoting) so generated string literals remain valid for all string inputs.
[logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ BigQuery queries with trailing backslash filter fail compilation.
- ❌ Dashboard filters on backslash-containing values return errors.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Note that `Database.compile_sqla_query` at
`superset/models/core.py:815-823` compiles
any SQLAlchemy `Select` using `qry.compile(engine,
compile_kwargs={"literal_binds":
True})`, so all engines (including BigQuery) inline literal values when
Superset generates
SQL.
2. Observe in `superset/db_engine_specs/base.py:1945-1965` that
`BaseEngineSpec.select_star` calls `sql = database.compile_sqla_query(qry,
table.catalog,
table.schema)`, so BigQuery-based datasets use this path to build the final
SQL string for
things like data preview and many Explore queries.
3. For a BigQuery database (URI starting with `bigquery://` so the engine
uses
`BigQueryDialect` defined in `superset/db_engine_specs/bigquery.py`), build
a query with a
string filter whose value ends in a backslash, e.g. a SQLAlchemy expression
`column("trailer") == "foo\\"`, and execute it through Superset (or in a
test mirroring
`tests/unit_tests/db_engine_specs/test_bigquery.py:152-161` by compiling with
`dialect=BigQueryDialect(), compile_kwargs={"literal_binds": True}`).
4. During compilation, SQLAlchemy emits a `BindParameter` for the filter
value and the
BigQuery-specific compiler override `compile_bind_param_bigquery` at
`superset/db_engine_specs/bigquery.py:17-27` runs the current code:
`if kw.get("literal_binds") and isinstance(element.value, str): val =
element.value.replace("'", "\\'"); return f"'{val}'"`.
For a value `"foo\\"`, this produces the literal `'foo\'` in the final
SQL, where the
`\'` at the end escapes the closing quote, yielding an invalid or
misparsed BigQuery
string literal and causing the BigQuery API to return a 400 syntax error
when Superset
executes the query.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=aae22f62dc754aa6a1df81961e3ea1a1&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=aae22f62dc754aa6a1df81961e3ea1a1&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/db_engine_specs/bigquery.py
**Line:** 53:55
**Comment:**
*Logic Error: The custom literal compiler only escapes single quotes,
but it leaves existing backslashes untouched before wrapping the value in
quotes. For inputs containing backslashes (especially values ending with `\`),
this can produce invalid BigQuery SQL like an escaped closing quote, causing
query failures and potentially corrupting literal boundaries. Escape
backslashes before apostrophes (or use the dialect literal processor and then
transform quoting) so generated string literals remain valid for all string
inputs.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40455&comment_hash=92404786dd52a27e99bd973928308536e228cd4e3b10aa9b398adf6201dc07f0&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40455&comment_hash=92404786dd52a27e99bd973928308536e228cd4e3b10aa9b398adf6201dc07f0&reaction=dislike'>👎</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
