codeant-ai-for-open-source[bot] commented on code in PR #38925:
URL: https://github.com/apache/superset/pull/38925#discussion_r3003669699
##########
tests/unit_tests/jinja_context_test.py:
##########
@@ -507,6 +507,27 @@ def test_where_in() -> None:
assert where_in(["O'Malley's"]) == "('O''Malley''s')"
+def test_where_in_bigquery_apostrophe() -> None:
+ """
+ Test that BigQuery dialect uses backslash escaping for apostrophes
+ instead of double-apostrophe escaping, which causes syntax errors.
+
+ See: https://github.com/apache/superset/issues/35857
+ """
+ try:
+ from sqlalchemy_bigquery import BigQueryDialect
+
+ where_in = WhereInMacro(BigQueryDialect())
+ result = where_in(["Armando's"])
+ # BigQuery requires backslash escaping, not double-apostrophe
+ assert "''" not in result, (
+ f"BigQuery should use backslash escaping, got double-apostrophe:
{result}"
+ )
+ assert "\\'" in result or "Armando" in result
Review Comment:
**Suggestion:** The final assertion in this test is too weak: by allowing
`"Armando"` as an alternative condition, the test will still pass if the
generated SQL contains an unescaped apostrophe like `Armando's` (which is
invalid BigQuery SQL), so the test does not actually enforce the stated
requirement that BigQuery use backslash escaping for apostrophes. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ BigQuery filters with apostrophes can regress without test failing.
- ⚠️ Dashboards using where_in on BigQuery may break silently.
- ⚠️ Regression risk for issue #35857 remains insufficiently covered.
```
</details>
```suggestion
assert "\\'" in result, (
f"BigQuery should escape apostrophes with backslash: {result}"
)
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Inspect `test_where_in_bigquery_apostrophe` in
`tests/unit_tests/jinja_context_test.py`
lines 510–527, where `result =
WhereInMacro(BigQueryDialect())(["Armando's"])` and the
assertions `assert "''" not in result` and `assert "\\'" in result or
"Armando" in result`
are evaluated.
2. Note from `superset/jinja_context.py:190–233` that
`WhereInMacro.__call__` builds a
parenthesized IN list using the SQLAlchemy dialect's literal rendering, and
from
`superset/db_engine_specs/bigquery.py:125–147` that the intended behavior is
to escape
apostrophes with backslash for BigQuery.
3. Consider an invalid-but-plausible BigQuery literal output such as `result
=
"('Armando's')"` (no double-apostrophe, no backslash escaping), which would
cause a
BigQuery syntax error in real queries, but still satisfies the current test
conditions: it
contains no `"''"` substring and does contain `"Armando"`.
4. Evaluate the current test logic with this `result` value: the first
assertion (`"''"
not in result`) passes because there are no doubled quotes, and the second
assertion
(`"\\'" in result or "Armando" in result`) passes because `"Armando"` is
present, so
`pytest` would report the test as passing even though the BigQuery
requirement
(backslash-escaped apostrophes) is violated.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/jinja_context_test.py
**Line:** 526:526
**Comment:**
*Logic Error: The final assertion in this test is too weak: by allowing
`"Armando"` as an alternative condition, the test will still pass if the
generated SQL contains an unescaped apostrophe like `Armando's` (which is
invalid BigQuery SQL), so the test does not actually enforce the stated
requirement that BigQuery use backslash escaping for apostrophes.
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.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38925&comment_hash=afeb824b4d1c8617a93a43a9b94f9b06dd89a5e0b5e107d884f3b7ff30de97cb&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38925&comment_hash=afeb824b4d1c8617a93a43a9b94f9b06dd89a5e0b5e107d884f3b7ff30de97cb&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]
