Skip to content

fix(sqllab): format_sql to apply db dialect by database_id#39393

Merged
michael-s-molina merged 5 commits into
apache:masterfrom
justinpark:fix--missing-engine-on-format-sql
Apr 16, 2026
Merged

fix(sqllab): format_sql to apply db dialect by database_id#39393
michael-s-molina merged 5 commits into
apache:masterfrom
justinpark:fix--missing-engine-on-format-sql

Conversation

@justinpark
Copy link
Copy Markdown
Member

@justinpark justinpark commented Apr 15, 2026

SUMMARY

Fixes #39100
In the SQL Lab, during the format_sql process, the engine information of the current database was not being passed, which caused the formatting to always fall back to the basic dialect. With the latest update (#36277), the database_id is now being passed, so the code has been updated to extract the engine value based on the database_id and pass it accordingly.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before:

before--dialect.mov

After:

after--dialect.mov

TESTING INSTRUCTIONS

  1. Connect Superset 6.0.0 (or 6.1rc1) to a Trino database (any version)
  2. Open SQL Lab, ensure "Format SQL" is ON
  3. Run:
    SELECT date_diff('day', DATE '2023-01-01', current_date)
  4. Observe error:
    TrinoUserError: Function 'datediff' not registered

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

@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review Bot commented Apr 15, 2026

Code Review Agent Run #207d89

Actionable Suggestions - 0
Additional Suggestions - 1
  • superset/sqllab/api.py - 1
    • Misleading Comment · Line 243-243
      The comment states 'Process Jinja templates if template_params are provided' but the code requires both template_params and database_id to be present. This could mislead developers about the conditions for template processing.
      Code suggestion
       @@ -243,1 +243,1 @@
      -            # Process Jinja templates if template_params are provided
      +            # Process Jinja templates if template_params and database_id are provided
Review Details
  • Files reviewed - 2 · Commit Range: 32c7555..32c7555
    • superset/sqllab/api.py
    • tests/integration_tests/sql_lab/api_tests.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

@github-actions github-actions Bot added the api Related to the REST API label Apr 15, 2026
@dosubot dosubot Bot added change:backend Requires changing the backend sqllab Namespace | Anything related to the SQL Lab labels Apr 15, 2026
Comment thread superset/sqllab/api.py
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 15, 2026

Codecov Report

❌ Patch coverage is 75.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.47%. Comparing base (0d91f5e) to head (745c466).
⚠️ Report is 23 commits behind head on master.

Files with missing lines Patch % Lines
superset/sqllab/api.py 75.00% 2 Missing and 1 partial ⚠️

❌ Your project status has failed because the head coverage (99.79%) is below the target coverage (100.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #39393      +/-   ##
==========================================
- Coverage   64.48%   64.47%   -0.01%     
==========================================
  Files        2556     2557       +1     
  Lines      132855   132924      +69     
  Branches    30860    30866       +6     
==========================================
+ Hits        85665    85700      +35     
- Misses      45699    45733      +34     
  Partials     1491     1491              
Flag Coverage Δ
hive 39.95% <0.00%> (+<0.01%) ⬆️
mysql 60.61% <75.00%> (-0.02%) ⬇️
postgres 60.69% <75.00%> (-0.02%) ⬇️
presto 41.74% <0.00%> (+<0.01%) ⬆️
python 62.27% <75.00%> (-0.02%) ⬇️
sqlite 60.32% <75.00%> (-0.02%) ⬇️
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.

@justinpark justinpark changed the title fix(sqllab: format_sql to apply db dialect by database_id fix(sqllab): format_sql to apply db dialect by database_id Apr 15, 2026
@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 15, 2026

Deploy Preview for superset-docs-preview ready!

Name Link
🔨 Latest commit 6820726
🔍 Latest deploy log https://app.netlify.com/projects/superset-docs-preview/deploys/69e00e9364ffe7000862fd75
😎 Deploy Preview https://deploy-preview-39393--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 15, 2026

Code Review Agent Run #c301d8

Actionable Suggestions - 0
Review Details
  • Files reviewed - 2 · Commit Range: 32c7555..745c466
    • superset/sqllab/api.py
    • tests/integration_tests/sql_lab/api_tests.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

@michael-s-molina michael-s-molina merged commit 0b51e9c into apache:master Apr 16, 2026
64 of 65 checks passed
michael-s-molina pushed a commit that referenced this pull request Apr 16, 2026
rusackas pushed a commit that referenced this pull request Apr 17, 2026
- theming.mdx: document brandAppName theme token (PR #37370) — controls
  app name in browser title/nav/emails, takes precedence over APP_NAME config
- cache.mdx: document SUPERSET_CACHE_WARMUP_USER config key (PR #38449) —
  controls the user account Selenium WebDriver authenticates as for thumbnail
  rendering and cache warmup; update selenium → Selenium capitalization
- security.mdx: document missing SQL Lab RBAC permissions (PR #36263) —
  can_estimate_query_cost and can_format_sql must be explicitly granted
- sql-templating.mdx: document Jinja support in calculated columns (PR #37791)
  with examples; add tip that "Format SQL" is Jinja-aware and dialect-specific
  (PRs #36277, #39393)
- creating-your-first-dashboard.mdx: document dashboard tab URLs (#38660),
  auto-refresh (#37459), "Last queried at" timestamp (#36934), and tab
  selection when saving charts to dashboards (#36332)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rusackas pushed a commit that referenced this pull request Apr 17, 2026
Cover remaining 6.1 features across existing and new pages:

MCP server:
- Add MCP_PARSE_REQUEST_ENABLED to configuration reference
- Add Audit Events section (MCP tool calls appear in Action Log)
- Add Tool Pagination section documenting cursor-based pagination (#37674)

Using AI with Superset:
- Expand Available Tools Reference into categorized sections covering
  all new tools added in the MCP tool library expansion
- Document preview-first workflow for generate_chart / update_chart

Creating Your First Dashboard:
- AG Grid server-side column filters (#35683): filter types, AND/OR logic,
  pagination interaction
- Time Shift for AG Grid Interactive Table (#37072)
- Dynamic currency formatting via currency_code_column dataset field (#36416)
- ECharts option editor in Explore for JSON overrides (#37868)
- Table chart: export behavior with search filter active (#36281)
- Dataset folders: organizing datasets into groups (#36239)
- PWA file handler: opening CSV/XLS/Parquet from OS file manager (#36191)
- Share database connection option when adding a new database (#37940)

Exploring Data:
- Dialect-aware Format SQL (applies selected database dialect) (#39393)
- SQL Lab tips section and time range natural language expressions
  (consolidates content from batch 4 for master branch)

Importing/Exporting:
- Dashboard import overwrite behavior: charts are replaced not duplicated (#36551)
- UUID in REST API POST responses for dataset/chart/dashboard (#37806)

New pages:
- docs/docs/using-superset/embedding.mdx: embedded SDK quick start,
  resolvePermalinkUrl callback (#36924),
  DISABLE_EMBEDDED_SUPERSET_LOGOUT feature flag (#37537),
  URL parameters, guest token security notes
- docs/admin_docs/configuration/aws-iam.mdx: cross-account IAM
  authentication for Aurora and Redshift via STS AssumeRole (#37585),
  configuration reference, trust policy setup guide

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rusackas pushed a commit that referenced this pull request Apr 22, 2026
- theming.mdx: document brandAppName theme token (PR #37370) — controls
  app name in browser title/nav/emails, takes precedence over APP_NAME config
- cache.mdx: document SUPERSET_CACHE_WARMUP_USER config key (PR #38449) —
  controls the user account Selenium WebDriver authenticates as for thumbnail
  rendering and cache warmup; update selenium → Selenium capitalization
- security.mdx: document missing SQL Lab RBAC permissions (PR #36263) —
  can_estimate_query_cost and can_format_sql must be explicitly granted
- sql-templating.mdx: document Jinja support in calculated columns (PR #37791)
  with examples; add tip that "Format SQL" is Jinja-aware and dialect-specific
  (PRs #36277, #39393)
- creating-your-first-dashboard.mdx: document dashboard tab URLs (#38660),
  auto-refresh (#37459), "Last queried at" timestamp (#36934), and tab
  selection when saving charts to dashboards (#36332)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
qfcwell pushed a commit to qfcwell/superset that referenced this pull request May 12, 2026
@github-actions github-actions Bot added 🍒 6.1.0 Cherry-picked to 6.1.0 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels labels May 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api Related to the REST API 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels change:backend Requires changing the backend size/M sqllab Namespace | Anything related to the SQL Lab 🍒 6.1.0 Cherry-picked to 6.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(trino): SQL formatter rewrites Trino-native functions to invalid names (date_diff→datediff, to_hex→hex) via sqlglot dialect mismatch

2 participants