Skip to content

fix(SqlLab): enhance SQL formatting with Jinja template support.#36277

Merged
rusackas merged 4 commits into
apache:masterfrom
LuisSanchez:fix/sql-formatting-jinja-template-support
Dec 8, 2025
Merged

fix(SqlLab): enhance SQL formatting with Jinja template support.#36277
rusackas merged 4 commits into
apache:masterfrom
LuisSanchez:fix/sql-formatting-jinja-template-support

Conversation

@LuisSanchez
Copy link
Copy Markdown
Contributor

SUMMARY

Enable Jinja-aware SQL formatting by passing database_id/template_params from the UI and processing templates server-side before formatting.
This is used on the experimental Parameters feature of the SQL Labs.

Details

  • Backend:

    • POST /api/v1/sqllab/format_sql/ now accepts database_id and template_params (JSON string) via FormatQueryPayloadSchema.
    • Processes Jinja templates (via get_template_processor) before running SQLScript.format.
  • Frontend (SQL Lab):

    • formatQuery includes database_id and template_params (stringified when needed) from the up-to-date query editor state in the request body.
  • Tests:

    • Add unit tests for formatQuery payload construction and dispatch behavior.
    • Add integration test verifying Jinja template resolution in format_sql response.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before

93622-error-jinja-formatting-sql-formatting.mov

After

93622-jinja-formatting-sql-formatting.mov

TESTING INSTRUCTIONS

  1. Navigate to SQL Lab.
  2. Make sure the examples DB is selected.
  3. Click on the three ellipses on the bottom right > Parameters.
  4. Set {"tbl": "\"Vehicle Sales\""} as the parameter value.
  5. Run select * from {{tbl}} and validate the query works.
  6. Click on the three ellipses on the bottom right > Format SQL.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags: ENABLE_TEMPLATE_PROCESSING
  • 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 Nov 25, 2025

Code Review Agent Run #e66591

Actionable Suggestions - 0
Additional Suggestions - 3
  • tests/integration_tests/sql_lab/api_tests.py - 1
    • Missing return type annotation public function · Line 291-291
      Add return type annotation `-> None` to method `test_format_sql_request_with_jinja` and add docstring.
      Code suggestion
       @@ -291,1 +291,1 @@
      -    def test_format_sql_request_with_jinja(self):
      +    def test_format_sql_request_with_jinja(self) -> None:
      +        """Test SQL formatting with Jinja template processing."""
  • superset/sqllab/api.py - 1
    • Missing trailing comma in function call · Line 261-261
      Add a trailing comma after the `**template_params` argument in the `process_template` method call on line 261.
      Code suggestion
       @@ -259,7 +259,7 @@
                            if template_params:
                                template_processor = get_template_processor(database=database)
                                sql = template_processor.process_template(
      -                            sql, **template_params
      +                            sql, **template_params,
                                )
  • superset/sqllab/schemas.py - 1
    • Trailing comma missing in field definition · Line 52-52
      Add a trailing comma after the `database_id` field definition on line 52 for consistency with code style conventions.
      Code suggestion
       @@ -51,3 +51,3 @@
            engine = fields.String(required=False, allow_none=True)
            database_id = fields.Integer(
      -        required=False, allow_none=True, metadata={"description": "The database id"}
      +        required=False, allow_none=True, metadata={"description": "The database id"},
            )
Review Details
  • Files reviewed - 5 · Commit Range: 0ee3762..0ee3762
    • superset-frontend/src/SqlLab/actions/sqlLab.js
    • superset-frontend/src/SqlLab/actions/sqlLab.test.js
    • superset/sqllab/api.py
    • superset/sqllab/schemas.py
    • tests/integration_tests/sql_lab/api_tests.py
  • Files skipped - 0
  • Tools
    • Eslint (Linter) - ✔︎ Successful
    • 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 Default Agent 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 Nov 25, 2025
@dosubot dosubot Bot added the sqllab Namespace | Anything related to the SQL Lab label Nov 25, 2025
Comment on lines -917 to +919
const { sql } = getUpToDateQuery(getState(), queryEditor);
const qe = getUpToDateQuery(getState(), queryEditor);
const { sql, dbId, templateParams } = qe;
const body = { sql };
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.

Can we still do this inline? are we using the variable qe anywhere else?

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.

Of course, and you're right, there are no more uses of the var qe.

Comment thread superset-frontend/src/SqlLab/actions/sqlLab.js Outdated
Comment thread superset/sqllab/api.py Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 26, 2025

Codecov Report

❌ Patch coverage is 75.00000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.99%. Comparing base (76d897e) to head (fb5243b).
⚠️ Report is 3132 commits behind head on master.

Files with missing lines Patch % Lines
superset/sqllab/api.py 71.42% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #36277      +/-   ##
==========================================
+ Coverage   60.48%   67.99%   +7.50%     
==========================================
  Files        1931      636    -1295     
  Lines       76236    46832   -29404     
  Branches     8568     5084    -3484     
==========================================
- Hits        46114    31842   -14272     
+ Misses      28017    13712   -14305     
+ Partials     2105     1278     -827     
Flag Coverage Δ
hive 43.74% <12.50%> (-5.42%) ⬇️
javascript ?
mysql 67.09% <75.00%> (?)
postgres 67.14% <75.00%> (?)
presto 47.33% <12.50%> (-6.47%) ⬇️
python 67.95% <75.00%> (+4.45%) ⬆️
sqlite 66.76% <75.00%> (?)
unit 100.00% <ø> (+42.36%) ⬆️

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.

@LuisSanchez LuisSanchez requested a review from msyavuz November 27, 2025 23:41
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Dec 4, 2025

⚠️ DEPRECATED WORKFLOW ⚠️

@geido This workflow is deprecated! Please use the new Superset Showtime system instead:

Processing your ephemeral environment request here. Action: up. More information on how to use or configure ephemeral environments

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Dec 4, 2025

@geido Ephemeral environment spinning up at http://44.249.161.7:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.

@rusackas rusackas merged commit e7c0604 into apache:master Dec 8, 2025
73 checks passed
sadpandajoe pushed a commit that referenced this pull request Dec 15, 2025
@sadpandajoe sadpandajoe added the v6.0 Label added by the release manager to track PRs to be included in the 6.0 branch label Dec 15, 2025
Facyla pushed a commit to Facyla/superset-contrib that referenced this pull request Dec 16, 2025
isaac-jaynes-imperva pushed a commit to isaac-jaynes-imperva/superset that referenced this pull request Mar 3, 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 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
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 size/L sqllab Namespace | Anything related to the SQL Lab v6.0 Label added by the release manager to track PRs to be included in the 6.0 branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants