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: [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]
