rusackas opened a new pull request, #42739:
URL: https://github.com/apache/superset/pull/42739
### SUMMARY
TDD PR for #40465 ("Superset streaming export bug"). The issue reports two
regressions in the streaming CSV export path introduced by #35478: (1) SQL sent
to the engine skips `Database.mutate_sql_based_on_config()`, so any
`SQL_QUERY_MUTATOR` never runs (the issue's repro shows this crashing Trino on
a trailing semicolon), and (2) user impersonation is allegedly dropped because
the streaming path "acquires its engine without context."
Independently verified both theories against current `master`:
- **Bug 1 (SQL mutation) confirmed.** `_execute_query_and_stream`
(`superset/commands/streaming_export/base.py`) calls
`engine.connect().execute(text(sql))` directly. The non-streaming path
(`Database.get_df` → `_execute_sql_with_mutation_and_logging`) calls
`self.mutate_sql_based_on_config(...)` on every statement before executing —
the streaming path skips this entirely, independent of any specific database
engine. Added a **red** test pinning this.
- **Bug 2 (impersonation) does not reproduce.** The issue cites a
`get_sqla_engine_with_context(user_name=...)` used by "every other execution
site" — no such function exists anywhere in the codebase. Both the streaming
and non-streaming paths call the same `Database.get_sqla_engine(catalog=,
schema=)`, which resolves the acting user internally via `get_effective_user()`
→ `get_username()` → `flask.g.user.username`.
`BaseStreamingCSVExportCommand.run()` already captures the full `flask.g` dict
(including `g.user`) before entering the deferred generator, and
`csv_generator()` restores it via `preserve_g_context` before
`_execute_query_and_stream` runs — so the acting user is correctly available at
engine-acquisition time. Added a **green** test pinning this (rules out this
half of the issue's theory rather than confirming it).
### TESTING INSTRUCTIONS
```
pytest tests/unit_tests/commands/sql_lab/streaming_export_command_test.py -k
"mutator or impersonat" -v
```
`test_streaming_export_applies_sql_mutator` fails on `master` (documents Bug
1). `test_streaming_export_preserves_impersonated_user_context` passes (rules
out Bug 2).
### ADDITIONAL INFORMATION
- [x] Has associated issue: #40465
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
