fix(bigquery): avoid ESCAPE clause in column-value typeahead search (SC-121408)

[eschutho]

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:

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)
  • 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

[bito-code-review]

Code Review Agent Run #53d44b

Actionable Suggestions - 0

Review Details

• Files reviewed - 2 · Commit Range: bf2103e..bf2103e

  • superset/models/helpers.py
  • tests/unit_tests/models/helpers_test.py
• Files skipped - 0
• Tools

  • MyPy (Static Code Analysis) - ✔︎ Successful
  • Astral Ruff (Static Code Analysis) - ✔︎ Successful
  • Whispers (Secret Scanner) - ✔︎ Successful
  • Detect-secrets (Secret Scanner) - ✔︎ Successful

---

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

• /review - Manually triggers an incremental AI Review.

• /review full - 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

• Customize agent settings
• Review rules
• General documentation
• FAQ

AI Code Review powered by

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.56%. Comparing base (a7dab4a) to head (bf2103e).
⚠️ Report is 6 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #44429      +/-   ##
==========================================
- Coverage   80.57%   80.56%   -0.01%     
==========================================
  Files        2940     2940              
  Lines      175316   175312       -4     
  Branches    40696    40696              
==========================================
- Hits       141253   141247       -6     
- Misses      31398    31400       +2     
  Partials     2665     2665              

Flag	Coverage Δ
hive	37.25% <0.00%> (-0.01%)	⬇️
mysql	56.50% <100.00%> (-0.01%)	⬇️
postgres	56.51% <100.00%> (-0.01%)	⬇️
presto	39.14% <0.00%> (-0.01%)	⬇️
python	84.87% <100.00%> (-0.01%)	⬇️
sqlite	56.23% <100.00%> (-0.01%)	⬇️
unit	76.60% <100.00%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[eschutho]

@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.

[rebenitez1802]

Approve: correct, well-targeted fix. The reported bug is real — sqlalchemy-bigquery's BigQueryCompiler overrides visit_contains_op_binary but not visit_like_op_binary, so the old .like(pattern, escape="!") leaked a literal ESCAPE '!' clause into GoogleSQL, which rejects it. Switching to .contains(..., autoescape=True) is the right primitive: BigQuery renders wildcard-escaping in its own syntax (no ESCAPE keyword) while other engines keep their dialect-native LIKE ... ESCAPE. The removed escape_like_pattern/LIKE_ESCAPE_CHAR are fully unreferenced, the new sqlalchemy_bigquery test import is already available in the unit-test CI env (development.txt), the updated test is a genuine regression guard (fails on the old code, passes on the new), and the security model is intact (value still reaches SQL via literal_binds single-quote escaping; wildcards remain neutralized). A few non-blocking notes below.

🟢 Low — Private Sentry issue ID committed into public source
The new comment at tests/unit_tests/models/helpers_test.py:130 embeds a private preset-inc.sentry.io issue short-ID (it's absent on master, so this diff introduces it into permanent public git history). The comment reads fine without it:

    # Regression test: BigQuery's GoogleSQL has no ESCAPE keyword, so the
    # compiled predicate must not emit one.

🟢 Low — PR title/body carry private tracker references
The trailing (SC-…) in the title and the Shortcut/Sentry URLs in the description are private Preset references on a public PR; a squash-merge would also carry the story ID into the public commit message. Consider dropping the story ID from the title and the internal URLs from the body before merge.

🟢 Low — BigQuery: literal backslash in the search term matches the wrong rows
autoescape only escapes /, %, and _, and sqlalchemy-bigquery's _maybe_reescape leaves a raw backslash untouched — yet GoogleSQL uses \ as the LIKE escape character. So a search containing \ (e.g. c\d) compiles to a pattern where \d is read as an escaped d, matching cd rather than c\d; a lone or trailing \ similarly mis-escapes the wildcard. This is not a regression — the pre-PR BigQuery path was fully broken by the syntax error, so this is a strict improvement — and there's no security impact (results stay within a column the user can already query). Fine to merge as-is; optionally pre-escape backslashes before .contains, or note the limitation.