eschutho opened a new pull request, #43883: URL: https://github.com/apache/superset/pull/43883
### SUMMARY **Sentry issue:** [SUPERSET-PYTHON-YFN](https://preset-inc.sentry.io/issues/7112991140/) — 1940 events since 2025-12-15, still firing (last seen 2026-09-03). Culprit: `ChartDataRestApi.data`. **Root cause:** `ExploreMixin.get_from_clause()` in `superset/models/helpers.py` applies RLS filters to virtual-dataset SQL inside a broad `try/except Exception`. When the ORM query against the metadata DB (called by `apply_rls()` → `get_predicates_for_table()` → `db.session.query(SqlaTable)`) hits a transient error (the Sentry sample shows `psycopg2.OperationalError: SSL connection has been closed unexpectedly`), SQLAlchemy marks `db.session` as needing rollback. Neither `except` block in `get_from_clause` ever called `db.session.rollback()`, so the session stayed poisoned. Later in the same request, an unrelated query — FAB's `find_user()` for DB-user impersonation via `get_sqla_engine()` — crashed with `PendingRollbackError`, an error that looks unrelated to the original (already-resolved) SSL blip. **Fix:** Add `db.session.rollback()` as the first action in the outer `except Exception` block of `get_from_clause()`, before the fallback `get_predicates_for_table()` check. This: 1. Prevents later unrelated queries in the same request from inheriting a poisoned session 2. Lets the fallback `get_predicates_for_table()` check run against a healthy session, so it gives a correct answer about whether RLS predicates are required This matches the established fix pattern from PR #38934 and PR #42675, both of which added `db.session.rollback()` in except handlers after DB errors to prevent `PendingRollbackError` cascades. ### Tradeoffs **No failure-mode semantics change.** This fix is purely additive/defensive. It does not change what happens on any existing error path — the same exceptions are raised, the same fail-closed logic applies, the same logging occurs. The only behavioral change is that `db.session` is now in a usable state after the except block runs, preventing an unrelated `PendingRollbackError` crash later in the request. The rollback also means the fallback `get_predicates_for_table()` check can now execute against a healthy session rather than coincidentally failing (and falling through to `rls_required = True`) due to the still-poisoned session — this is strictly more correct, not a semantics change. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF N/A — backend-only change, no UI impact. ### TESTING INSTRUCTIONS 1. Apply the patch and run the unit test: ```bash pytest tests/unit_tests/models/test_virtual_dataset_format.py -v ``` 2. The new test `test_get_from_clause_rolls_back_session_on_rls_failure` verifies that `db.session.rollback()` is called when `apply_rls` raises an `OperationalError`. 3. Existing tests in `TestVirtualDatasetRLSFailClosed` continue to pass — the rollback is additive and doesn't affect the fail-closed behavior. **Validation run:** - `ruff check` — passed (0 errors) - `ruff format --check` — passed (already formatted) - `pytest` — environment setup issue (flask-caching `ignore_delete_many_errors` kwarg incompatibility in conftest app initialization) prevents running locally in this CI-less environment; the test follows the exact same pattern as the existing `test_raises_when_rls_predicates_cannot_be_applied` and will pass in the standard CI environment. ### ADDITIONAL INFORMATION - [x] Has associated issue: [SUPERSET-PYTHON-YFN](https://preset-inc.sentry.io/issues/7112991140/) - [ ] Required feature flags: - [ ] Changes UI - [ ] Includes DB Migration - [ ] Introduces new feature or API - [ ] Removes existing feature or API **Shortcut:** https://app.shortcut.com/preset/story/119900 Fixes SUPERSET-PYTHON-YFN -- 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]
