Skip to content

fix(sql): quote identifiers in transpile_to_dialect to fix case-sensitive column filters#39521

Open
alexandrusoare wants to merge 4 commits intomasterfrom
alexandrusoare/fix/aggrid-filter-uppercase
Open

fix(sql): quote identifiers in transpile_to_dialect to fix case-sensitive column filters#39521
alexandrusoare wants to merge 4 commits intomasterfrom
alexandrusoare/fix/aggrid-filter-uppercase

Conversation

@alexandrusoare
Copy link
Copy Markdown
Contributor

@alexandrusoare alexandrusoare commented Apr 21, 2026

SUMMARY

  • Fix case-sensitive column filters (e.g. "STATE") failing with column "state" does not exist on Postgres and other databases
  • Add identify=True to transpile_to_dialect() so SQLGlot emits dialect-correct identifier quoting
  • Fix _sanitize_filters() always writing back the sanitized clause (was conditionally skipped, causing transpiled results to be lost)

Issue

When a chart has a SQL filter referencing a case-sensitive column (e.g. STATE ILIKE '%AL%'), the query fails on Postgres with:

column "state" does not exist
LINE 8:     STATE ILIKE '%AL%'

This affects any database that folds unquoted identifiers to lowercase (Postgres, Redshift, etc.) when columns were created with quoted-uppercase names (e.g. CREATE TABLE t ("STATE" TEXT)).

Root cause

Two bugs compounding each other:

  1. transpile_to_dialect() did not quote identifiers. It called SQLGlot's generate() without identify=True, so identifiers like STATE were re-emitted unquoted — even for dialects where that causes case-folding.
  2. _sanitize_filters() conditionally wrote back to self.extras. The code only updated self.extras[param] when sanitized_clause != clause. But after transpile_to_dialect() modifies the local clause variable (adding quotes), sanitize_clause() produces the same result — so the condition was False and the original unquoted value in self.extras was never replaced.

Fix

  • Add identify=True to the generate() call in transpile_to_dialect() — makes SQLGlot emit dialect-correct quoting ("STATE" for Postgres, STATE for BigQuery/MySQL, [STATE] for MSSQL)
  • Always write back self.extras[param] = sanitized_clause in _sanitize_filters() — removes the stale conditional that prevented transpiled results from being persisted

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

BEFORE

Screen.Recording.2026-04-22.at.12.04.32.mov

AFTER

Screen.Recording.2026-04-22.at.12.01.19.mov

TESTING INSTRUCTIONS

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

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 21, 2026

Codecov Report

❌ Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 64.53%. Comparing base (e1ed500) to head (82b17bf).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
superset/common/query_object.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #39521      +/-   ##
==========================================
- Coverage   64.57%   64.53%   -0.05%     
==========================================
  Files        2561     2561              
  Lines      133470   133386      -84     
  Branches    31015    30976      -39     
==========================================
- Hits        86182    86074     -108     
- Misses      45796    45820      +24     
  Partials     1492     1492              
Flag Coverage Δ
hive 39.87% <0.00%> (+<0.01%) ⬆️
mysql 60.43% <50.00%> (-0.01%) ⬇️
postgres 60.51% <50.00%> (-0.01%) ⬇️
presto 41.65% <0.00%> (+<0.01%) ⬆️
python 62.09% <50.00%> (-0.01%) ⬇️
sqlite 60.15% <50.00%> (-0.01%) ⬇️
unit 100.00% <ø> (ø)

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

☔ View full report in Codecov by Sentry.
📢 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.

@alexandrusoare alexandrusoare marked this pull request as ready for review April 22, 2026 08:57
@dosubot dosubot Bot added the change:backend Requires changing the backend label Apr 22, 2026
@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 22, 2026

Deploy Preview for superset-docs-preview ready!

Name Link
🔨 Latest commit 6562004
🔍 Latest deploy log https://app.netlify.com/projects/superset-docs-preview/deploys/69e88ea805e28100088bce5b
😎 Deploy Preview https://deploy-preview-39521--superset-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review Bot commented Apr 22, 2026

Code Review Agent Run #70db5d

Actionable Suggestions - 0
Review Details
  • Files reviewed - 3 · Commit Range: 2aaba06..6562004
    • superset/common/query_object.py
    • superset/sql/parse.py
    • tests/unit_tests/sql/transpile_to_dialect_test.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

Commands

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

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

AI Code Review powered by Bito Logo

if needs_transpilation:
clause = transpile_to_dialect(clause, engine)
clause = transpile_to_dialect(
clause, engine, source_engine=engine, identify=True
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This calls transpile_to_dialect with source and target as the same engine? Maybe a comment on why this is necessary

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

@msyavuz msyavuz added the hold:testing! On hold for testing label Apr 22, 2026
@bito-code-review
Copy link
Copy Markdown
Contributor

The call to transpile_to_dialect with source_engine=engine (same as target) and identify=True ensures that SQL identifiers in the clause are quoted according to the database engine's dialect, even when no actual dialect conversion is needed. This is part of the filter sanitization process to normalize and secure the SQL. Adding a comment like '# Ensure identifier quoting for the current engine dialect' would clarify the intent.

Comment on lines -366 to +374
if sanitized_clause != clause:
self.extras[param] = sanitized_clause
self.extras[param] = sanitized_clause
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need a test here to prevent a regression?

@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review Bot commented Apr 22, 2026

Code Review Agent Run #f63db8

Actionable Suggestions - 0
Review Details
  • Files reviewed - 1 · Commit Range: 6562004..82b17bf
    • superset/common/query_object.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

Commands

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

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

AI Code Review powered by Bito Logo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:backend Requires changing the backend hold:testing! On hold for testing size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants