fix(sql): preserve optimizer hints when formatting semicolon-terminated statements with trailing comments by FrancescoCastaldi · Pull Request #43565 · apache/superset

FrancescoCastaldi · 2026-08-26T22:01:41Z

SUMMARY

In SQLStatement._parse, when a SQL script contained statements terminated by a semicolon followed by trailing -- comments (which sqlglot parses as a trailing exp.Semicolon statement), the comment relocation logic iterated over all AST nodes using .walk().

Because .walk() traverses AST subtrees in internal dictionary order rather than reverse SQL document rendering order, target was erroneously set to leaf nodes inside optimizer hint blocks (e.g. exp.Hint -> Identifier("query_timeout")), causing trailing comments to be injected directly inside optimizer hints (e.g. /*+ SET_VAR(query_timeout /* comment */ = 3000) */).

This PR fixes this by introducing _find_last_token_node(node), which:

Traverses SQL clauses in true reverse generation order (offset, limit, order, window, where, from, expressions, etc.).
Explicitly skips exp.Hint / non-trailing nodes so that comments are safely attached to the final token/clause of the statement.
Removes the @pytest.mark.xfail marker from test_sqlscript_format_preserves_optimizer_hint_block_with_semicolon in tests/unit_tests/sql/parse_tests.py and adds tests covering CTEs and optimizer hints.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A (SQL Parsing & formatting fix)

TESTING INSTRUCTIONS

Execute formatting on a query containing an optimizer hint and trailing comments with a terminating semicolon (e.g., in SQL Lab with StarRocks/MySQL):
```
SELECT /*+ SET_VAR(query_timeout = 3000) */ col1, col2
FROM my_table
LIMIT 100;

-- increase timeout for large scans
```
Verify that the query formats cleanly without inserting comments inside the /*+ ... */ hint block.

ADDITIONAL INFORMATION

Has associated issue: Fixes bug: SQL parser injects -- comments inside optimizer hint blocks (/*+ SET_VAR */), breaking StarRocks syntax #38189
Required feature flags:
Changes UI
Includes DB Migration
Introduces new feature or API
Removes existing feature or API

SUGGESTED LABELS

#bug:regression, data:sqllab, sql, validation:validated, P2

…ed statements with trailing comments

bito-code-review · 2026-08-26T22:01:51Z

Code Review Agent Run #056cc0

Actionable Suggestions - 0

Review Details

Files reviewed - 2 · Commit Range: 0ca8de0..0ca8de0
- superset/sql/parse.py
- tests/unit_tests/sql/parse_tests.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

AI Code Review powered by

bito-code-review · 2026-08-26T22:06:45Z

The flagged issue is correct. The current implementation of _find_last_token_node in superset/sql/parse.py uses the argument name window for exp.Select, but SQLGlot's Select AST uses windows (plural). Consequently, queries with WINDOW clauses are not correctly identified as the final clause, causing trailing comments to be attached to the wrong node.

To resolve this, update the clause_name tuple in _find_last_token_node to use windows instead of window:

# superset/sql/parse.py

        for clause_name in (
            "options",
            "settings",
            "format",
            "locks",
            "offset",
            "limit",
            "sort",
            "cluster",
            "distribute",
            "order",
            "windows",  # Updated from "window"
            "qualify",
            "having",
            "group",
            "where",
            "joins",
            "laterals",
            "from",
            "into",
            "expressions",
        ):

There are no other review comments on this PR.

superset/sql/parse.py

for clause_name in (
            "options",
            "settings",
            "format",
            "locks",
            "offset",
            "limit",
            "sort",
            "cluster",
            "distribute",
            "order",
            "windows",  # Updated from "window"
            "qualify",
            "having",
            "group",
            "where",
            "joins",
            "laterals",
            "from",
            "into",
            "expressions",
        ):

…line length

codecov · 2026-08-26T22:45:50Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.10%. Comparing base (a140e74) to head (96dc379).
⚠️ Report is 5 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #43565   +/-   ##
=======================================
  Coverage   79.10%   79.10%           
=======================================
  Files        2878     2878           
  Lines      165634   165659   +25     
  Branches    38294    38305   +11     
=======================================
+ Hits       131023   131050   +27     
  Misses      32123    32123           
+ Partials     2488     2486    -2

Flag	Coverage Δ
hive	`37.95% <12.00%> (-0.01%)`	⬇️
mysql	`57.69% <12.00%> (-0.02%)`	⬇️
postgres	`57.72% <12.00%> (-0.02%)`	⬇️
presto	`39.86% <12.00%> (-0.01%)`	⬇️
python	`83.68% <100.00%> (+<0.01%)`	⬆️
sqlite	`57.41% <12.00%> (-0.02%)`	⬇️
unit	`73.78% <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.

bito-code-review · 2026-08-27T01:13:57Z

Code Review Agent Run #36e83f

Actionable Suggestions - 0

Review Details

Files reviewed - 2 · Commit Range: 0ca8de0..78c1a8a
- superset/sql/parse.py
- tests/unit_tests/sql/parse_tests.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

AI Code Review powered by

…verage

bito-code-review · 2026-08-27T11:54:50Z

Code Review Agent Run #3b182a

Actionable Suggestions - 0

Review Details

Files reviewed - 1 · Commit Range: 78c1a8a..96dc379
- tests/unit_tests/sql/parse_tests.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

AI Code Review powered by

rusackas

Thanks for chasing down #38189! CI's green, and codeant's windows catch already landed in a later commit, so there's nothing outstanding there. LGTM, approving!

fix(sql): preserve optimizer hints when formatting semicolon-terminat…

0ca8de0

…ed statements with trailing comments

pull-request-size Bot added the size/M label Aug 26, 2026

codeant-ai-for-open-source Bot reviewed Aug 26, 2026

View reviewed changes

Comment thread superset/sql/parse.py Outdated

FrancescoCastaldi added 2 commits August 27, 2026 00:11

fix: use correct sqlglot arg name 'windows' (plural) for WINDOW clause

faeb61e

style: reduce cyclomatic complexity in _find_last_token_node and fix …

7db3985

…line length

pull-request-size Bot added size/L and removed size/M labels Aug 26, 2026

ci: re-trigger CI matrix to resolve transient network timeouts

78c1a8a

rusackas requested review from betodealmeida and rusackas August 26, 2026 22:53

FrancescoCastaldi added 2 commits August 27, 2026 10:34

test(sql): add unit tests for _find_last_token_node to ensure 100% co…

2dc08ba

…verage

style: fix line length in parse_tests.py for ruff compliance

96dc379

rusackas approved these changes Sep 9, 2026

View reviewed changes

rusackas merged commit b1185d8 into apache:master Sep 9, 2026
73 checks passed

Navigation Menu

Repository navigation

fix(sql): preserve optimizer hints when formatting semicolon-terminated statements with trailing comments - #43565

fix(sql): preserve optimizer hints when formatting semicolon-terminated statements with trailing comments#43565
rusackas merged 6 commits into
apache:masterfrom
FrancescoCastaldi:fix/sql-optimizer-hints-comments

FrancescoCastaldi commented Aug 26, 2026 •

edited

Loading

Uh oh!

bito-code-review Bot commented Aug 26, 2026 •

edited

Loading

Code Review Agent Run #056cc0

Uh oh!

Uh oh!

bito-code-review Bot commented Aug 26, 2026

Uh oh!

codecov Bot commented Aug 26, 2026 •

edited

Loading

Uh oh!

bito-code-review Bot commented Aug 27, 2026 •

edited

Loading

Code Review Agent Run #36e83f

Uh oh!

bito-code-review Bot commented Aug 27, 2026 •

edited

Loading

Code Review Agent Run #3b182a

Uh oh!

rusackas left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

FrancescoCastaldi commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

SUGGESTED LABELS

Uh oh!

bito-code-review Bot commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Agent Run #056cc0

Uh oh!

Uh oh!

bito-code-review Bot commented Aug 26, 2026

Uh oh!

codecov Bot commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

bito-code-review Bot commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Agent Run #36e83f

Uh oh!

bito-code-review Bot commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Agent Run #3b182a

Uh oh!

rusackas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

FrancescoCastaldi commented Aug 26, 2026 •

edited

Loading

bito-code-review Bot commented Aug 26, 2026 •

edited

Loading

codecov Bot commented Aug 26, 2026 •

edited

Loading

bito-code-review Bot commented Aug 27, 2026 •

edited

Loading

bito-code-review Bot commented Aug 27, 2026 •

edited

Loading