fix(versioning): scope stream_results to the activity SELECTs instead of mutating the session connection by mikebridge · Pull Request #44261 · apache/superset

mikebridge · 2026-09-14T20:57:56Z

SUMMARY

The activity fetch executed its streaming SELECTs via db.session.connection().execution_options(stream_results=True) — and Connection.execution_options mutates the connection in place, permanently flipping the request session into server-side-cursor mode. On PostgreSQL, every later statement on the same request — Continuum's version_transaction INSERT, SAVEPOINTs, and notably the DBEventLogger write that follows any activity read — then gets wrapped in DECLARE ... CURSOR WITHOUT HOLD FOR ... and fails with psycopg2.errors.SyntaxError (the event write is swallowed by the logger's error handler, so activity-endpoint telemetry is silently lost on PG deployments today). Surfaced deterministically by sc-120488's tests, which write through the same session after get_activity — CI traceback: run 34888050672 / job 104123743848. The fix rides the option on each SELECT statement instead — dialect-neutral, one-line, no connection mutation.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A — connection semantics.

TESTING INSTRUCTIONS

pytest tests/integration_tests/versioning/activity_write_after_read_tests.py — an activity read followed by an INSERT + commit on the same session (the DBEventLogger shape). Control-verified on PostgreSQL 16: reverting the fix fails the test with the DECLARE-CURSOR syntax error; green on SQLite/MySQL either way (only PG maps stream_results to server-side cursors).

ADDITIONAL INFORMATION

Has associated issue: sc-120955
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

🤖 Generated with Claude Code

https://claude.ai/code/session_01JRLEJS4mUqKBoPjSjviKUW

… of mutating the session connection (sc-120955) Connection.execution_options mutates the connection in place, so the activity fetch permanently flipped the request session into server-side-cursor mode. On PostgreSQL every later statement on the request — Continuum's version_transaction INSERT, SAVEPOINTs, and the DBEventLogger write that follows any activity read — gets wrapped in DECLARE ... CURSOR FOR and fails with a syntax error; the event write is swallowed by the logger's handler, so activity-endpoint telemetry is silently lost on PG deployments today. The option now rides each SELECT statement. Regression test: an activity read followed by a write on the same session (the DBEventLogger shape) — control-verified on PostgreSQL 16 (reverting the fix fails the test; SQLite is unaffected either way since only PG turns stream_results into server-side cursors). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JRLEJS4mUqKBoPjSjviKUW

bito-code-review · 2026-09-14T20:58:08Z

Code Review Agent Run #5f2352

Actionable Suggestions - 0

Additional Suggestions - 1

tests/integration_tests/versioning/activity_write_after_read_tests.py - 1
- Disallow Any type · Line 35-35
  
  Replace `Any` with a concrete type, e.g., `User` from `flask_appbuilder.security.sqla.models`. Also add the import.

Review Details

Files reviewed - 2 · Commit Range: 345fd33..345fd33
- superset/versioning/activity/queries.py
- tests/integration_tests/versioning/activity_write_after_read_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

codecov · 2026-09-14T21:03:20Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.39%. Comparing base (4c77b0b) to head (bfd6f58).

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #44261      +/-   ##
==========================================
- Coverage   80.39%   80.39%   -0.01%     
==========================================
  Files        2932     2932              
  Lines      174525   174525              
  Branches    40548    40548              
==========================================
- Hits       140318   140313       -5     
- Misses      31538    31543       +5     
  Partials     2669     2669

Flag	Coverage Δ
hive	`37.34% <ø> (+0.18%)`	⬆️
mysql	`56.60% <ø> (ø)`
postgres	`56.62% <ø> (ø)`
presto	`39.22% <ø> (ø)`
python	`84.75% <ø> (-0.01%)`	⬇️
sqlite	`56.32% <ø> (ø)`
unit	`76.35% <ø> (-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.

aminghadersohi

Reverting the two production lines reds the new test with the exact DECLARE ... CURSOR WITHOUT HOLD FOR INSERT failure on Continuum's version_transaction insert, so fix and regression test both hold up. One note inline on what the test doesn't pin.

bito-code-review · 2026-09-16T08:59:26Z

The observation is correct. Moving stream_results=True from the connection level to the statement level is the necessary fix to prevent the connection from being permanently mutated into server-side cursor mode. Using a before_cursor_execute event listener to inspect cursor.name or the statement itself is a robust way to verify that the streaming option is correctly applied to the specific statement rather than the connection.

superset/versioning/activity/queries.py

result = (
                db.session.connection()
-                .execution_options(stream_results=True)
-                .execute(stmt)
+                .execute(stmt.execution_options(stream_results=True))
                .mappings()
            )

Answers discussion_r4024252815: observe named cursors even when stream_results is removed, while asserting connection options remain clean and later writes succeed. Replace the admin helper Any return with FAB User as requested by Bito.

netlify · 2026-09-16T22:58:32Z

✅ Deploy Preview for superset-docs-preview ready!

Name	Link
🔨 Latest commit	`bfd6f58`
🔍 Latest deploy log	https://app.netlify.com/projects/superset-docs-preview/deploys/6aab327315ad6b0008f61826
😎 Deploy Preview	https://deploy-preview-44261--superset-docs-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

bito-code-review · 2026-09-17T01:57:38Z

Code Review Agent Run #9e85ca

Actionable Suggestions - 0

Additional Suggestions - 1

tests/integration_tests/versioning/activity_write_after_read_tests.py - 1
- Cursor-name timing assertion · Line 101-101
  
  `all(streamed_cursors)` likely always fails on PostgreSQL: in SQLAlchemy's psycopg2 dialect the server-side cursor name is set inside `do_execute`, which fires *after* `before_cursor_execute`, so `cursor.name` is still `None` when `observe_change_select` runs. Check `context.execution_options.get("stream_results")` instead, which is available at that point.

Review Details

Files reviewed - 1 · Commit Range: 345fd33..bfd6f58
- tests/integration_tests/versioning/activity_write_after_read_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

mikebridge · 2026-09-17T04:49:34Z

Prepared by Codex at Mike's request. Following up on Bito's cursor-name timing note (#44261 (comment)): could we retain the named-cursor assertion? SQLAlchemy 2.0.52's _PGExecutionContext_common_psycopg.create_server_side_cursor constructs the identifier and calls self._dbapi_connection.cursor(ident) before execution; it is not first assigned inside do_execute. I inspected that installed implementation and exercised its cursor-creation call.

The actual PostgreSQL CI job at tested merge 268ec0434d3ab355587ea03d4679019fe38534c0 logs tests/integration_tests/versioning/activity_write_after_read_tests.py . (not skipped), then 2791 passed, 165 skipped: https://github.com/apache/superset/actions/runs/35166198716/job/105027834221 . Published head remains bfd6f58fc7d2d2631a2b8809ba4881a2a6cd0e06. The earlier negative controls and fix are linked in #44261 (comment) . An execution-options-only assertion would weaken the regression the reviewer requested. This disposes the timing objection as a false positive; it does not claim reviewer approval.

… of mutating the session connection (#44261) Co-authored-by: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit c426186)

pull-request-size Bot added the size/L label Sep 14, 2026

mikebridge mentioned this pull request Sep 14, 2026

feat(versioning): render the starting version as the oldest history row for charts and dashboards #44258

Merged

3 of 9 tasks

aminghadersohi self-requested a review September 16, 2026 06:26

aminghadersohi reviewed Sep 16, 2026

View reviewed changes

Comment thread superset/versioning/activity/queries.py Resolved

chore: merge repaired master to refresh PR 44261 CI

bfd6f58

mikebridge mentioned this pull request Sep 17, 2026

fix(versioning): mount the dashboard version-history panel inside the content area, below the header #44255

Merged

3 of 9 tasks

richardfogaca approved these changes Sep 17, 2026

View reviewed changes

richardfogaca merged commit c426186 into apache:master Sep 17, 2026
76 checks passed

villebro pushed a commit that referenced this pull request Sep 20, 2026

fix(versioning): scope stream_results to the activity SELECTs instead…

38499ae

… of mutating the session connection (#44261) Co-authored-by: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit c426186)

Navigation Menu

Repository navigation

fix(versioning): scope stream_results to the activity SELECTs instead of mutating the session connection - #44261

fix(versioning): scope stream_results to the activity SELECTs instead of mutating the session connection#44261
richardfogaca merged 3 commits into
apache:masterfrom
mikebridge:sc-120955-stream-results-statement-scope

mikebridge commented Sep 14, 2026

Uh oh!

bito-code-review Bot commented Sep 14, 2026 •

edited

Loading

Code Review Agent Run #5f2352

Uh oh!

codecov Bot commented Sep 14, 2026 •

edited

Loading

Uh oh!

aminghadersohi left a comment

Uh oh!

Uh oh!

bito-code-review Bot commented Sep 16, 2026

Uh oh!

netlify Bot commented Sep 16, 2026 •

edited

Loading

Uh oh!

bito-code-review Bot commented Sep 17, 2026 •

edited

Loading

Code Review Agent Run #9e85ca

Uh oh!

mikebridge commented Sep 17, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

mikebridge commented Sep 14, 2026

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Uh oh!

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

Uh oh!

Code Review Agent Run #5f2352

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

aminghadersohi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bito-code-review Bot commented Sep 16, 2026

Uh oh!

netlify Bot commented Sep 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for superset-docs-preview ready!

Uh oh!

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

Uh oh!

Code Review Agent Run #9e85ca

Uh oh!

mikebridge commented Sep 17, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bito-code-review Bot commented Sep 14, 2026 •

edited

Loading

codecov Bot commented Sep 14, 2026 •

edited

Loading

netlify Bot commented Sep 16, 2026 •

edited

Loading

bito-code-review Bot commented Sep 17, 2026 •

edited

Loading