You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "eschutho (via GitHub)" <gi...@apache.org> on 2026/09/18 15:11:58 UTC
[PR] fix(bigquery): avoid ESCAPE clause in column-value typeahead search (SC-121408) [superset]
eschutho opened a new pull request, #44429:
URL: https://github.com/apache/superset/pull/44429
### SUMMARY
Fixes a `DatabaseError: Syntax error: Expected end of input but got keyword ESCAPE` thrown on **BigQuery** datasets whenever a user types into a filter's column-value typeahead search box.
- **Sentry:** https://preset-inc.sentry.io/issues/7737898154/ (SUPERSET-PYTHON-176K) — 119 events, 0 users, single-customer BigQuery project.
- **Shortcut story:** https://app.shortcut.com/preset/story/121408
**Root cause**
The typeahead path is `DatasourceRestApi.get_column_values` → `SqlaTable.values_for_column` → `build_like_predicate` (added by #43518, "feat(filters): search filter values server-side in Explore"). `build_like_predicate` built its predicate with:
```python
pattern = f"%{escape_like_pattern(search)}%".lower()
return sa.func.lower(expr).like(pattern, escape=LIKE_ESCAPE_CHAR)
```
SQLAlchemy's base compiler (`visit_like_op_binary` / `visit_not_like_op_binary`) always appends a literal `ESCAPE '<char>'` clause when an escape char is passed to `.like()`. `sqlalchemy-bigquery`'s `BigQueryCompiler` does **not** override those two methods — it only overrides `visit_contains_op_binary` / `visit_startswith_op_binary` / `visit_endswith_op_binary` (via a `_maybe_reescape` helper that pops the `escape` modifier and re-encodes wildcards using BigQuery's backslash-escape convention, since GoogleSQL's `LIKE` has no `ESCAPE` keyword). So the literal `ESCAPE '!'` leaked straight into the SQL sent to BigQuery, which rejects it — exactly the reported syntax error.
Reproduced by compiling the predicate against `BigQueryDialect()`:
```
SELECT DISTINCT lower(`my_table`.`col`) LIKE '%%foo%%' ESCAPE '!' AS `column_values` FROM `my_table`
```
**Fix**
Switch `build_like_predicate` to SQLAlchemy's `.contains(..., autoescape=True)` operator instead of a hand-rolled `.like(pattern, escape=...)` string. `.contains()` compiles through `visit_contains_op_binary`, which BigQueryCompiler **does** override, so BigQuery renders wildcard-escaping in its own supported syntax (backslash-escaping, no `ESCAPE` clause) while other engines keep their dialect-native `LIKE ... ESCAPE` predicate.
Compiled output after the fix:
```
PG : lower(c) LIKE '%' || '50/%%/_off' || '%' ESCAPE '/'
BQ : lower(`c`) LIKE '%' || '50\%%\_off' || '%' (no ESCAPE clause)
MYSQL : lower(c) LIKE concat('%', '50/%%/_off', '%') ESCAPE '/'
```
The now-unused `LIKE_ESCAPE_CHAR` constant and `escape_like_pattern()` helper are removed (`build_like_predicate` was their only caller — confirmed by grep).
### TESTING INSTRUCTIONS
Manual: on a BigQuery dataset in Explore, open a filter on a string column and type into the value typeahead — values now load instead of erroring.
Automated (`tests/unit_tests/models/helpers_test.py`):
- Removed `test_escape_like_pattern` (function no longer exists).
- Updated `test_build_like_predicate_is_case_insensitive_and_escaped` — asserts the postgres compilation still lower-cases + escapes and still emits an `ESCAPE` clause (other engines unaffected), and **compiles against `sqlalchemy_bigquery.BigQueryDialect()` asserting `"ESCAPE" not in` the SQL** (the regression test for this bug).
- Updated `test_values_for_column_search` — `.contains()` compiles to `... LIKE '%' || 'ali' || '%'` on sqlite, so it now asserts both `LIKE` and `'ali'` appear.
Results:
- `pytest tests/unit_tests/models/helpers_test.py -v` → **178 passed**
- `pytest tests/unit_tests/models/` → **526 passed**
- `ruff check` + `ruff format --check` on both files → clean
- `pre-commit run --files ...` (mypy, ruff, pylint) → passed
### Tradeoffs
- **No failure-mode / semantics change.** The predicate still performs the same case-insensitive substring match on every engine; only *how* the SQL is generated changes, not *what* it does. BigQuery goes from erroring to matching correctly; all other engines produce equivalent SQL.
- `escape_like_pattern`'s custom `!`-based wildcard escaping is replaced by SQLAlchemy's dialect-default escape char (e.g. `/` on postgres/mysql). This is purely an internal SQL-generation detail and is not observable to users.
### Follow-ups
None expected. There are currently no other BigQuery `LIKE`-with-escape call sites (confirmed by grep — `build_like_predicate` was the sole user of the removed helpers). If any new `LIKE ... ESCAPE` call sites are added in the future, they should be checked against BigQuery for the same `ESCAPE`-clause incompatibility.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(bigquery): avoid ESCAPE clause in column-value typeahead search (SC-121408) [superset]
Posted by "eschutho (via GitHub)" <gi...@apache.org>.
eschutho commented on PR #44429:
URL: https://github.com/apache/superset/pull/44429#issuecomment-5732132704
@rebenitez1802 — routing this to you. It's the same `superset/models/helpers.py` bug-cleanup lane you've been carrying end-to-end (most recently #44172), and the fix is a SQLAlchemy-compiler dialect quirk — squarely your #43864 territory: `.like(escape=…)` renders a literal `ESCAPE` clause via the base compiler, which `sqlalchemy-bigquery`'s `BigQueryCompiler` doesn't override, so it leaks into GoogleSQL; the PR switches to `.contains(autoescape=True)`, which the dialect *does* override.
Pipeline-authored (eschutho, Sentry burndown SUPERSET-PYTHON-176K / SC-121408), self-reviewed, CI green-bar, small diff (+34/-45, two files, new regression test). Two things worth confirming in your read: (1) that `build_like_predicate` is actually the predicate path `DatasourceRestApi.get_column_values` dispatches — the Sentry culprit — and (2) that the `.contains(autoescape=True)` swap stays a genuine no-op on postgres/mysql/sqlite.
Your tracker load is the highest on the roster right now — happy to re-route if you're underwater.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(bigquery): avoid ESCAPE clause in column-value typeahead search (SC-121408) [superset]
Posted by "eschutho (via GitHub)" <gi...@apache.org>.
eschutho merged PR #44429:
URL: https://github.com/apache/superset/pull/44429
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(bigquery): avoid ESCAPE clause in column-value typeahead search (SC-121408) [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #44429:
URL: https://github.com/apache/superset/pull/44429#issuecomment-5732089184
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/44429?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `0%` with `1 line` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 57.37%. Comparing base ([`a7dab4a`](https://app.codecov.io/gh/apache/superset/commit/a7dab4a5519cfc1cbfd2e2e1c83622cc1110ccde?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`bf2103e`](https://app.codecov.io/gh/apache/superset/commit/bf2103ec4ec06017e2317686823a72e8ed9ee63a?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/44429?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [superset/models/helpers.py](https://app.codecov.io/gh/apache/superset/pull/44429?src=pr&el=tree&filepath=superset%2Fmodels%2Fhelpers.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvbW9kZWxzL2hlbHBlcnMucHk=) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/44429?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
> :exclamation: There is a different number of reports uploaded between BASE (a7dab4a) and HEAD (bf2103e). Click for more details.
>
> <details><summary>HEAD has 83 uploads less than BASE</summary>
>
>| Flag | BASE (a7dab4a) | HEAD (bf2103e) |
>|------|------|------|
>|python|73|1|
>|presto|12|1|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #44429 +/- ##
===========================================
- Coverage 80.57% 57.37% -23.21%
===========================================
Files 2940 2940
Lines 175316 175312 -4
Branches 40696 40696
===========================================
- Hits 141253 100577 -40676
- Misses 31398 73775 +42377
+ Partials 2665 960 -1705
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/44429/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/44429/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [mysql](https://app.codecov.io/gh/apache/superset/pull/44429/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/44429/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/44429/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.14% <0.00%> (-0.01%)` | :arrow_down: |
| [python](https://app.codecov.io/gh/apache/superset/pull/44429/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.14% <0.00%> (-45.74%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/44429/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit](https://app.codecov.io/gh/apache/superset/pull/44429/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/44429?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(bigquery): avoid ESCAPE clause in column-value typeahead search (SC-121408) [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44429:
URL: https://github.com/apache/superset/pull/44429#issuecomment-5732026370
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org