Skip to content

fix(streaming-export): route SQL through mutate_sql_based_on_config#40495

Open
Abdulrehman-PIAIC80387 wants to merge 1 commit into
apache:masterfrom
Abdulrehman-PIAIC80387:fix/streaming-export-mutate-sql-40465
Open

fix(streaming-export): route SQL through mutate_sql_based_on_config#40495
Abdulrehman-PIAIC80387 wants to merge 1 commit into
apache:masterfrom
Abdulrehman-PIAIC80387:fix/streaming-export-mutate-sql-40465

Conversation

@Abdulrehman-PIAIC80387
Copy link
Copy Markdown
Contributor

SUMMARY

Addresses Bug 1 of #40465. The streaming CSV export path (added in #35478) sends raw chart SQL directly to connection.execute(text(sql)) without first calling database.mutate_sql_based_on_config(). Every other Superset execution site (SQL Lab, chart-data JSON, alerts, validators, helpers) routes SQL through that mutator so engine-spec / config transforms are applied.

The most visible consequence is on Trino: the SQL Superset generates for a chart ends with LIMIT N;, and Trino's HTTP statement endpoint rejects trailing semicolons with mismatched input ';'. Expecting: <EOF>. Because the streaming response has already flushed headers by the time the exception fires, the user receives an HTTP 200 with the sentinel __STREAM_ERROR__: Export failed... written into what should have been their CSV file.

Fix

In superset/commands/streaming_export/base.py::_execute_query_and_stream, run the SQL through the same mutator the non-streaming paths use:

+ mutated_sql = merged_database.mutate_sql_based_on_config(sql)
  with merged_database.get_sqla_engine(...) as engine:
      with engine.connect() as connection:
          result_proxy = connection.execution_options(
              stream_results=True
-         ).execute(text(sql))
+         ).execute(text(mutated_sql))

TESTING INSTRUCTIONS

pytest tests/unit_tests/commands/chart/streaming_export_command_test.py \
       tests/unit_tests/commands/sql_lab/streaming_export_command_test.py -v
  • New regression test test_streaming_sql_is_mutated_before_execute confirms mutate_sql_based_on_config is called with the raw chart SQL and that the mutated string is what connection.execute receives.
  • Both shared test fixtures stub the mutator as a passthrough so the existing 27 streaming-export tests continue to pass unchanged.

Result locally: 28 / 28 pass.

Out of scope (Bug 2 from #40465)

The issue also reports that user impersonation is bypassed on the streaming path — every export shows up in the Trino query log as the service principal regardless of which Superset user triggered it. Verifying that requires reproducing against an impersonation-enabled Trino, which is out of scope for this PR. Tracking separately so the security-sensitive change gets its own review.

ADDITIONAL INFORMATION

@dosubot dosubot Bot added data:connect:trino Related to Trino viz:charts:export Related to exporting charts labels May 28, 2026
@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review Bot commented May 28, 2026

Code Review Agent Run #8705a4

Actionable Suggestions - 0
Filtered by Review Rules

Bito filtered these suggestions based on rules created automatically for your feedback. Manage rules.

  • tests/unit_tests/commands/sql_lab/streaming_export_command_test.py - 1
Review Details
  • Files reviewed - 3 · Commit Range: 8bd3b55..8bd3b55
    • superset/commands/streaming_export/base.py
    • tests/unit_tests/commands/chart/streaming_export_command_test.py
    • tests/unit_tests/commands/sql_lab/streaming_export_command_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

@codecov
Copy link
Copy Markdown

codecov Bot commented May 28, 2026

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 64.17%. Comparing base (c73106b) to head (8bd3b55).

Files with missing lines Patch % Lines
superset/commands/streaming_export/base.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #40495      +/-   ##
==========================================
- Coverage   64.17%   64.17%   -0.01%     
==========================================
  Files        2592     2592              
  Lines      139299   139300       +1     
  Branches    32347    32347              
==========================================
  Hits        89395    89395              
- Misses      48367    48368       +1     
  Partials     1537     1537              
Flag Coverage Δ
hive 39.20% <0.00%> (-0.01%) ⬇️
mysql 58.70% <0.00%> (-0.01%) ⬇️
postgres 58.78% <0.00%> (-0.01%) ⬇️
presto 40.88% <0.00%> (-0.01%) ⬇️
python 60.34% <0.00%> (-0.01%) ⬇️
sqlite 58.42% <0.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.

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

Labels

data:connect:trino Related to Trino size/M viz:charts:export Related to exporting charts

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant