You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "aminghadersohi (via GitHub)" <gi...@apache.org> on 2026/09/16 13:36:15 UTC
[PR] perf(deletion-retention): one window pass for repeat predicate [superset]
aminghadersohi opened a new pull request, #44349:
URL: https://github.com/apache/superset/pull/44349
### SUMMARY
Builds on #44262 (open, not yet merged — this branch includes its commit as a
prerequisite baseline, so the diff below contains both). #44262 rewrote the
purge-audit "repeat block" candidacy check from a correlated nested-EXISTS
predicate to a window-function form, computing five `LAG(...) OVER (...)`
columns (timestamp, count, coded-count, min/max reason) over the batch's
blocked-timestamp groups. All five share an identical
`PARTITION BY (entity_type, entity_uuid) ORDER BY ts` spec.
PostgreSQL recognizes the repeated window spec and evaluates it once, but
MySQL 8 does not merge the five inline specs — each materializes its own
temporary table, so the query pays for roughly five sequential passes over
the batch's timestamp groups instead of one.
This PR replaces the five inline `OVER (...)` clauses with one SQL-level
named `WINDOW w AS (...)` clause, referenced by all five `LAG` columns
(SQLAlchemy Core has no construct for a named `WINDOW` clause, so this is a
small raw-text fragment — the query otherwise remains fully within Core).
MySQL now evaluates the window once; PostgreSQL's plan is unaffected since
the join structure and predicates are unchanged.
An alternative was also tried and rejected: `LAG` only the previous
timestamp (one window column) and fetch its aggregates via an equality join
back to a second instance of the groups derived table. Measured on
PostgreSQL with a single-entity batch scope, the planner underestimates that
second derived table's row count and picks a Nested Loop over an unindexed
`Materialize` of it — an O(batch × history) comparison, ~10x slower than the
baseline in testing. This reproduces the same failure shape as a prior
groups-to-groups join regression from #44262's own history, just via a
different join path, so it was dropped in favor of the named-`WINDOW` form,
which keeps the already-safe join structure entirely unchanged.
### TESTING INSTRUCTIONS
- `pytest tests/unit_tests/commands/deletion_retention/test_prune_audit.py`
— 66 passed, unchanged from #44262 (no test edits in this PR).
- `pytest tests/integration_tests/deletion_retention/prune_audit_tests.py`
— 35 passed (34 passed + 1 skipped on SQLite, which is expected: that test
is MySQL/PostgreSQL-only), including the 40-seed randomized equivalence
test, run against SQLite, a real MySQL 8 container, and a real
PostgreSQL 17 container.
- Ad-hoc timing comparison (not an in-repo script; a one-off harness run
against local Docker containers, five reps, median reported), seeding one
entity with 6,000 blocked rows, `ANALYZE`, MySQL forced to
`REPEATABLE READ`, timing the locked-recheck SELECT shape used by
`_delete_batch`:
- MySQL 8, batch 500: five-pass baseline ~170–236ms → one-pass ~85–127ms
(meets the ≤100ms target).
- MySQL 8, batch 50: ~156–175ms → ~67–94ms.
- PostgreSQL 17, batch 500: ~382–427ms → ~375–415ms (no regression;
within run-to-run noise).
- PostgreSQL 17, batch 50: ~67–71ms → ~68–69ms (no regression).
These PostgreSQL absolute numbers are higher than in prior discussion on
this predicate, which likely reflects this measurement environment/seed
shape rather than this change — the baseline (`git stash` this PR's commit,
keep #44262's) shows the same elevated numbers on the same setup, so the
before/after comparison is apples-to-apples even though the absolute
figures differ from the earlier report.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] 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
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] perf(deletion-retention): one window pass for repeat predicate [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44349:
URL: https://github.com/apache/superset/pull/44349#issuecomment-5706427862
<!-- Bito Reply -->
The performance concern raised in the review is valid. The current implementation uses separate lists for entity types and UUIDs to scope the re-check, which can lead to a Cartesian product (cross-product) if not handled carefully, potentially causing the database to scan a large number of unrelated entity histories while holding the coordination lock.
To resolve this, you should ensure that the entity scoping uses paired tuples (e.g., `(entity_type, entity_uuid)`) rather than separate lists. This forces the database to filter by the specific entity pairs identified during the discovery phase, preventing the cross-product and keeping the scan bounded by the batch size.
### Proposed Fix
In `superset/commands/deletion_retention/prune_audit.py`, update the scoping logic to use paired tuples:
```python
# Instead of separate lists, use a list of tuples for the scope:
scope_entities = list(set(zip(entity_types, entity_uuids)))
# Then, in the SQL query, use a tuple comparison:
# WHERE (entity_type, entity_uuid) IN :scope_entities
```
Would you like me to check the other comments on this PR and implement fixes for them as well?
**superset/commands/deletion_retention/prune_audit.py**
```
# Instead of separate lists, use a list of tuples for the scope:
scope_entities = list(set(zip(entity_types, entity_uuids)))
# Then, in the SQL query, use a tuple comparison:
# WHERE (entity_type, entity_uuid) IN :scope_entities
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] perf(deletion-retention): one window pass for repeat predicate [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44349:
URL: https://github.com/apache/superset/pull/44349#issuecomment-5712150523
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**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 <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] perf(deletion-retention): one window pass for repeat predicate [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #44349:
URL: https://github.com/apache/superset/pull/44349#issuecomment-5698464304
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/44349?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `9.52381%` with `19 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 56.23%. Comparing base ([`23bc3de`](https://app.codecov.io/gh/apache/superset/commit/23bc3de0ca3224101aa9f5f1f5d4f147a1161be1?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`c58684d`](https://app.codecov.io/gh/apache/superset/commit/c58684dc45d675e1ec6fa51a7d5ed7485dc8664b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/44349?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...uperset/commands/deletion\_retention/prune\_audit.py](https://app.codecov.io/gh/apache/superset/pull/44349?src=pr&el=tree&filepath=superset%2Fcommands%2Fdeletion_retention%2Fprune_audit.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvY29tbWFuZHMvZGVsZXRpb25fcmV0ZW50aW9uL3BydW5lX2F1ZGl0LnB5) | 9.52% | [19 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/44349?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
> :exclamation: There is a different number of reports uploaded between BASE (23bc3de) and HEAD (c58684d). Click for more details.
>
> <details><summary>HEAD has 20 uploads less than BASE</summary>
>
>| Flag | BASE (23bc3de) | HEAD (c58684d) |
>|------|------|------|
>|python|19|1|
>|hive|3|1|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #44349 +/- ##
===========================================
- Coverage 80.28% 56.23% -24.05%
===========================================
Files 2928 2928
Lines 174116 174130 +14
Branches 40402 40403 +1
===========================================
- Hits 139784 97930 -41854
- Misses 31675 75694 +44019
+ Partials 2657 506 -2151
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/44349/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/44349/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `37.15% <9.52%> (-0.01%)` | :arrow_down: |
| [mysql](https://app.codecov.io/gh/apache/superset/pull/44349/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/44349/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/44349/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [python](https://app.codecov.io/gh/apache/superset/pull/44349/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `37.15% <9.52%> (-47.54%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/44349/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit](https://app.codecov.io/gh/apache/superset/pull/44349/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/44349?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] perf(deletion-retention): one window pass for repeat predicate [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44349:
URL: https://github.com/apache/superset/pull/44349#issuecomment-5968489622
**Bito Automatic Review Skipped – PR Already Merged**<br/><br/>Bito scheduled an automatic review for this pull request, but the review was skipped because this PR was merged before the review could be run.<br/>No action is needed if you didn't intend to review it. To get a review, you can type `/review` in a comment and save it
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] perf(deletion-retention): one window pass for repeat predicate [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #44349:
URL: https://github.com/apache/superset/pull/44349#issuecomment-5705564139
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | eda957c9bde4c548b78235d3237fc639c1bc24be |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6aab1a03bf829a0008e2c3c3 |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-44349--superset-docs-preview.netlify.app](https://deploy-preview-44349--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DcpLCoAgEADQu8w6YVQK7QYdY_xUQqmYIRHdPXdv8V7IJZ25wgwwgCkU7d59WcYF6hFZvsvmGd0uVNZCdKn153w-0rO4PiciwwmlWZXQhIjKCyuthO8H) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-44349--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] perf(deletion-retention): one window pass for repeat predicate [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on PR #44349:
URL: https://github.com/apache/superset/pull/44349#issuecomment-5703096356
Both suggestions were valid and both are fixed. Neither was pre-existing — the branch introduced both conditions, so they were in scope for it.
**Unused scope query under lock** — confirmed. `_evidence_predicates` accepted `scope_entities` and never referenced it; only `_duplicate_predicates` and `_operational_predicates` consume it, via `_repeats_an_earlier_block`, the one predicate that binds a literal entity scope. The evidence category's guards are all correlated per-row probes, so the `DISTINCT entity_type, entity_uuid` lookup was dead work on the locked path — which is the path this change exists to keep short. `master` has no scope lookup at all, so the branch added it.
Categories now declare whether their predicates take a scope, and the kwarg is omitted rather than passed empty, so a builder that needs a scope can't silently receive an absent one. The unused parameter is gone from `_evidence_predicates`.
The re-check SQL is unchanged. Compiling each category's predicates under both the MySQL and PostgreSQL dialects renders byte-identical statements before and after, and the old `_evidence_predicates` output was already invariant to whatever `scope_entities` it was handed. The only behavioural delta is one fewer statement under the lock, for one category — strictly less work, never more.
**Vacuous test comparison** — confirmed. `_evidence_predicates` never reaches `_repeats_an_earlier_block`, so patching the legacy form left `rewritten["evidence"]` and `legacy["evidence"]` computed by identical code and the assertion could not fail. The docstring's claim to cover "evidence bounds" overstated it.
Kept the comparison rather than dropping it, but relabelled: it's a control that asserts the rewrite stays confined to its two categories — if `evidence` ever differed, the rewrite had leaked. The docstring now says it proves isolation, not equivalence, and no longer claims evidence-bound coverage.
Unit and integration suites for this area pass unchanged (110 unit, 120 integration, 2 skipped as engine-specific), including the seeded equivalence sweep.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] perf(deletion-retention): one window pass for repeat predicate [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44349:
URL: https://github.com/apache/superset/pull/44349#issuecomment-5702126671
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**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 <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] perf(deletion-retention): one window pass for repeat predicate [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi merged PR #44349:
URL: https://github.com/apache/superset/pull/44349
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] perf(deletion-retention): one window pass for repeat predicate [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #44349:
URL: https://github.com/apache/superset/pull/44349#issuecomment-5769770769
This is stacked on #44262, which hasn't merged yet, so we can't really take this until that one lands first. CodeAnt's cross-product concern looks resolved with a solid rebuttal. Once #44262's in and this rebases cleanly on `master`, happy to take another look.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] perf(deletion-retention): one window pass for repeat predicate [superset]
Posted by "mikebridge (via GitHub)" <gi...@apache.org>.
mikebridge commented on PR #44349:
URL: https://github.com/apache/superset/pull/44349#issuecomment-5708834902
Prepared by Codex at Mike's request; dependency coordination only, no changes to your branch. The prerequisite #44262 is published at `db03401048de06282835bab5e86bf37db2f407ff` with repaired green CI and ratified default50/max100. Could we reconcile this named-window optimization with that exact baseline, retaining MySQL<8/unknown-version fallback, normalized MariaDB dispatch, candidate self-filters, cross-type UUID/differential tests, and unlocked entity discovery? The new cross-product and whole-history discovery feedback is acknowledged on #44262 as performance limitations; neither that PR nor this coordination note claims O(batch), a production capacity guarantee, or live MySQL5.7/MariaDB validation. Your branch remains yours; I have not rewritten or absorbed the optimization.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] perf(deletion-retention): one window pass for repeat predicate [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on code in PR #44349:
URL: https://github.com/apache/superset/pull/44349#discussion_r4032586800
##########
superset/commands/deletion_retention/prune_audit.py:
##########
@@ -372,53 +374,139 @@ def _repeats_an_earlier_block(
survivor while its streak is current. It is also exempt from operational
age-out (see :func:`_operational_candidates`), so an operator force-purge
block is retained permanently — never pruned by either category.
+
+ P is the immediately preceding distinct blocked timestamp. A row repeats
+ only when P is in the current streak and both timestamp groups contain
+ solely its reason (including all-NULL groups). LAG over timestamp groups
+ supplies P and its reason counts without a group self-join.
+
+ All five ``LAG`` columns share one SQL-level named ``WINDOW w`` (a raw
+ text fragment: SQLAlchemy Core has no construct for a named ``WINDOW``
+ clause) instead of five separate ``LAG(...) OVER (...)`` expressions that
+ happen to repeat the same partition/order spec. PostgreSQL already
+ recognizes five identical inline window specs as one logical pass, but
+ MySQL 8 does not merge them — each materializes its own temporary table,
+ roughly five sequential passes over the batch's timestamp groups. A named
+ window is the SQL-level way to say "this is the same window" so MySQL
+ evaluates it once; PostgreSQL and SQLite (>= 3.25) accept the same syntax
+ unchanged (sc-120950).
+ An equality join back to a *second* instance of the timestamp-groups
+ derived table (fetching P's aggregates via
+ ``(entity_type, entity_uuid, ts = prev_ts)`` instead of four more ``LAG``
+ columns) was measured and rejected here: PostgreSQL's planner
+ misestimates the derived table's row count for a single-entity batch scope
+ and chooses a Nested Loop over an unindexed ``Materialize`` of the second
+ instance — an O(batch × history) comparison, the same failure shape as
+ the sc-120493 round-1 CTE/entity-only-merge regression, just via a
+ different join path. The named-``WINDOW`` shape keeps the exact join
+ structure already measured safe on PostgreSQL (sc-120493): only the
+ ``LAG`` columns' SQL text changes.
+
+ Keep the repeat-id query uncorrelated: sc-120493 measurements in
+ lock-hold-evidence/REPORT.md, Variant 2, showed MySQL repeatedly executing
+ per-row predecessor scalars. During re-check, scope blocked rows, timestamp
+ groups and boundaries with literal entity-type and UUID lists from the
+ fresh locked lookup. Their cross-product may include extra entity histories,
+ but candidacy remains restricted to the discovered ids and checked in SQL.
"""
- earlier: sa.FromClause = table.alias("earlier_block")
- between: sa.FromClause = table.alias("reason_change")
- reason_changed_between: sa.ColumnElement[bool] = sa.exists(
- sa.select(sa.literal(1))
- .select_from(between)
- .where(
- sa.and_(
- between.c.status == STATUS_BLOCKED,
- between.c.entity_type == table.c.entity_type,
- between.c.entity_uuid == table.c.entity_uuid,
- # Inclusive bounds: a differing-reason block sharing an
- # exact timestamp with either endpoint still breaks the run,
- # so a reason-transition row tied with a neighbour is
- # preserved as a run head rather than pruned as a repeat
- # (the same preserving-side tie rule the pending and evidence
- # guards use). Inclusive bounds only ever add boundaries —
- # i.e. only ever preserve more, never delete more.
- between.c.created_on >= earlier.c.created_on,
- between.c.created_on <= table.c.created_on,
- between.c.reason.is_distinct_from(table.c.reason),
- )
+ source: sa.Table = PurgeAuditLog.__table__
+ scope: list[sa.ColumnElement[bool]] = []
+ if scope_entities is not None:
+ types: list[str] = sorted({t for t, _ in scope_entities})
+ uuids: list[str] = sorted({u for _, u in scope_entities if u is not None})
+ scope = [source.c.entity_type.in_(types), source.c.entity_uuid.in_(uuids)]
Review Comment:
The cross-product here is bounded to the in-scope entities; it does not admit unrelated histories.
Emitted predicate, identical in all three derived tables and on both PostgreSQL and MySQL:
```sql
WHERE purge_audit_log.status = 'blocked'
AND purge_audit_log.entity_uuid IS NOT NULL
AND purge_audit_log.created_on <= :now
AND purge_audit_log.entity_type IN ('dashboards', 'slices', 'tables')
AND purge_audit_log.entity_uuid IN (...)
```
Two bounds:
1. `entity_type` is the table name of a registered `SoftDeleteMixin` subclass. That universe is exactly three values — `dashboards`, `slices`, `tables`. With `MAX_BATCH_SIZE = 500` the combination count tops out at 3 × 500 = 1,500. Reaching 250,000 would require 500 distinct entity types.
2. The two `IN` terms are ANDed, so the matched set is their intersection. `entity_uuid` is the entity's own `uuid4`, so it identifies exactly one entity and therefore one `entity_type`. `entity_uuid IN (≤500 uuids)` alone already bounds the scan to the in-scope entities' histories; the additional (type, uuid) combinations match zero rows, because `entity_type IN (...)` can only narrow the set, never widen it.
Measured on a heterogeneous batch — 3 entity types × 4 entities × 3 history rows in scope, plus 150 out-of-scope same-type histories present in the table:
```
scope pairs : 12
types x uuids combos : 3 x 12 = 36
rows matched cross-product : 36
rows matched paired : 36
identical row sets : True
extra rows from cross-prod : 0
```
`WHERE (entity_type, entity_uuid) IN (...)` selects the same rows. The existing conjunction also matches the leading columns of `ix_purge_audit_log_pruning (status, entity_type, entity_uuid, created_on)` directly.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] perf(deletion-retention): one window pass for repeat predicate [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44349:
URL: https://github.com/apache/superset/pull/44349#issuecomment-5698378021
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] perf(deletion-retention): one window pass for repeat predicate [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on PR #44349:
URL: https://github.com/apache/superset/pull/44349#issuecomment-5967066098
Both follow-ups from the approval are in 37dde86013: the window's ORDER BY and LAG columns now come from shared module constants, and a MySQL integration test runs EXPLAIN FORMAT=JSON and asserts a single windowing step carrying all LAG functions.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] perf(deletion-retention): one window pass for repeat predicate [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on PR #44349:
URL: https://github.com/apache/superset/pull/44349#issuecomment-5705549168
Good catch on the missing opt-out coverage — added in `eda957c9bd`, tests only.
Two tests in `tests/unit_tests/commands/deletion_retention/test_prune_audit.py`:
- `test_delete_batch_skips_the_locked_scope_lookup_when_the_category_opts_out` — the mirror of the existing scoped-path test. Asserts `recheck_predicates` is called as `(table)` with `scope_entities` **absent from kwargs**, not passed empty, and pins the statement order under the lock as `execute, rollback, lock, execute, execute, commit` — exactly one fewer execute between the lock and the commit than the scoped path takes.
- `test_only_the_evidence_category_opts_out_of_the_locked_scope_lookup` — pins the wiring, so the flag can't be declared and then not reach `_delete_batch`: `[(_duplicate_predicates, True), (_operational_predicates, True), (_evidence_predicates, False)]`.
Verified they can actually fail: dropping `needs_entity_scope=False` from the evidence category and forcing the scope lookup back on in `_delete_batch` turns both red (`At index 2 diff: (_evidence_predicates, True) != (_evidence_predicates, False)`); reverted, and `prune_audit.py` is byte-identical to the previous head. 112 passed in the directory (was 110), pre-commit clean.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] perf(deletion-retention): one window pass for repeat predicate [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on PR #44349:
URL: https://github.com/apache/superset/pull/44349#issuecomment-5709083222
Done — merged #44262 at `db03401048de06282835bab5e86bf37db2f407ff` (not rebased; this branch has submitted reviews, so history is preserved). New head is `7a4eacca93f0c7f2bd2e29fbb90e5c2687bfc5e9`.
Conflicts were in `prune_audit.py` and its two test modules. I resolved all three to your version wholesale, then re-applied only the named-window change on top, so your baseline is the substrate rather than something I merged around. The delta on top of `db03401048` is 3 files, +55/-6.
All five properties verified by reading the merged code, not by assuming the merge kept them:
1. **MySQL<8 / unknown-version fallback** — `_repeat_path()` returns `"legacy"` for `server_version_info is None` and for `< (8, 0)`; `test_r2_unknown_mysql_version_falls_back` passes.
2. **Normalized MariaDB dispatch** — `_mariadb_normalized_version_info` gated at `>= (10, 2)`, reached via `is_mariadb` under both the `mysql` and `mariadb` dialect names; `test_r2_mariadb_uses_normalized_version` passes on all 6 params including `5.5.5-10.6.12-MariaDB-0+deb11u1`.
3. **Candidate self-filters** — the `status`/`entity_uuid`/`created_on`/`trigger` gates are intact on `_legacy_repeats_an_earlier_block`, so the two helpers still agree in isolation and under `NOT`; `test_r2_repeat_helpers_preserve_ineligible_candidates` passes.
4. **Cross-type UUID / differential tests** — present and passing: `test_repeat_paths_isolate_entity_types_and_agree`, `test_repeat_dispatch_uses_metadata_server_version`, `test_window_repeat_predicate_executes_on_sqlite` (both helpers), and the integration `test_rewritten_repeat_predicate_matches_legacy_on_random_histories` with its window/legacy/fallback three-way comparison.
5. **Unlocked entity discovery** — unchanged: discovery runs before `acquire_coordination_lock`, scope pairs are read post-lock only for the categories that consume them, and `needs_entity_scope=False` still short-circuits the evidence category.
Your `_RecheckPredicates` protocol and the `_DuplicateRecheck`/`_OperationalRecheck`/`_EvidenceRecheck` adapters supersede the untyped `scope_kwargs` dict this branch was carrying, so I dropped ours and kept yours. `BATCH_SIZE = 50` / `MAX_BATCH_SIZE = 100` are as ratified.
The named window is applied to the window path only, so servers routed to the legacy correlated form never emit the clause. Compiling the locked re-check for all three categories before and after the merge, the SQL is byte-identical on both PostgreSQL and MySQL — the merge did not change what runs under the lock. The MySQL locked statement still shows one `WINDOW w`, five `OVER w`, zero inline `OVER (PARTITION ...)`.
Tests: 103 passed in `tests/unit_tests/commands/deletion_retention/test_prune_audit.py`, 31 in `tests/unit_tests/cli/deletion_retention_test.py`, 34 passed / 1 skipped in the integration suite (the skip is the REPEATABLE READ snapshot case, which needs MySQL or PG). CI is running at the new head.
I left the acknowledged cross-product and whole-history-discovery limitations alone, as you asked.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] perf(deletion-retention): one window pass for repeat predicate [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44349:
URL: https://github.com/apache/superset/pull/44349#issuecomment-5705326584
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**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 <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] perf(deletion-retention): one window pass for repeat predicate [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44349:
URL: https://github.com/apache/superset/pull/44349#issuecomment-5707149282
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**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 <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] perf(deletion-retention): one window pass for repeat predicate [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #44349:
URL: https://github.com/apache/superset/pull/44349#discussion_r4031914593
##########
superset/commands/deletion_retention/prune_audit.py:
##########
@@ -372,53 +374,139 @@ def _repeats_an_earlier_block(
survivor while its streak is current. It is also exempt from operational
age-out (see :func:`_operational_candidates`), so an operator force-purge
block is retained permanently — never pruned by either category.
+
+ P is the immediately preceding distinct blocked timestamp. A row repeats
+ only when P is in the current streak and both timestamp groups contain
+ solely its reason (including all-NULL groups). LAG over timestamp groups
+ supplies P and its reason counts without a group self-join.
+
+ All five ``LAG`` columns share one SQL-level named ``WINDOW w`` (a raw
+ text fragment: SQLAlchemy Core has no construct for a named ``WINDOW``
+ clause) instead of five separate ``LAG(...) OVER (...)`` expressions that
+ happen to repeat the same partition/order spec. PostgreSQL already
+ recognizes five identical inline window specs as one logical pass, but
+ MySQL 8 does not merge them — each materializes its own temporary table,
+ roughly five sequential passes over the batch's timestamp groups. A named
+ window is the SQL-level way to say "this is the same window" so MySQL
+ evaluates it once; PostgreSQL and SQLite (>= 3.25) accept the same syntax
+ unchanged (sc-120950).
+ An equality join back to a *second* instance of the timestamp-groups
+ derived table (fetching P's aggregates via
+ ``(entity_type, entity_uuid, ts = prev_ts)`` instead of four more ``LAG``
+ columns) was measured and rejected here: PostgreSQL's planner
+ misestimates the derived table's row count for a single-entity batch scope
+ and chooses a Nested Loop over an unindexed ``Materialize`` of the second
+ instance — an O(batch × history) comparison, the same failure shape as
+ the sc-120493 round-1 CTE/entity-only-merge regression, just via a
+ different join path. The named-``WINDOW`` shape keeps the exact join
+ structure already measured safe on PostgreSQL (sc-120493): only the
+ ``LAG`` columns' SQL text changes.
+
+ Keep the repeat-id query uncorrelated: sc-120493 measurements in
+ lock-hold-evidence/REPORT.md, Variant 2, showed MySQL repeatedly executing
+ per-row predecessor scalars. During re-check, scope blocked rows, timestamp
+ groups and boundaries with literal entity-type and UUID lists from the
+ fresh locked lookup. Their cross-product may include extra entity histories,
+ but candidacy remains restricted to the discovered ids and checked in SQL.
"""
- earlier: sa.FromClause = table.alias("earlier_block")
- between: sa.FromClause = table.alias("reason_change")
- reason_changed_between: sa.ColumnElement[bool] = sa.exists(
- sa.select(sa.literal(1))
- .select_from(between)
- .where(
- sa.and_(
- between.c.status == STATUS_BLOCKED,
- between.c.entity_type == table.c.entity_type,
- between.c.entity_uuid == table.c.entity_uuid,
- # Inclusive bounds: a differing-reason block sharing an
- # exact timestamp with either endpoint still breaks the run,
- # so a reason-transition row tied with a neighbour is
- # preserved as a run head rather than pruned as a repeat
- # (the same preserving-side tie rule the pending and evidence
- # guards use). Inclusive bounds only ever add boundaries —
- # i.e. only ever preserve more, never delete more.
- between.c.created_on >= earlier.c.created_on,
- between.c.created_on <= table.c.created_on,
- between.c.reason.is_distinct_from(table.c.reason),
- )
+ source: sa.Table = PurgeAuditLog.__table__
+ scope: list[sa.ColumnElement[bool]] = []
+ if scope_entities is not None:
+ types: list[str] = sorted({t for t, _ in scope_entities})
+ uuids: list[str] = sorted({u for _, u in scope_entities if u is not None})
+ scope = [source.c.entity_type.in_(types), source.c.entity_uuid.in_(uuids)]
Review Comment:
**Suggestion:** Separate type and UUID lists admit their full cross-product, so a heterogeneous batch can scan and sort up to 250,000 unrelated entity histories while holding the coordination lock. [performance]
**Assessment:** 🟠 `Major` · 🔁 `Occurrence: Rarely`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=899998ddee12484eaf090b62cfddfbd6&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=899998ddee12484eaf090b62cfddfbd6&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/commands/deletion_retention/prune_audit.py
**Line:** 415:417
**Comment:**
*Performance: Separate type and UUID lists admit their full cross-product, so a heterogeneous batch can scan and sort up to 250,000 unrelated entity histories while holding the coordination lock.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44349&comment_hash=0bd80f2e5b74766a526dd436ea04447d2896848992aa1710995b172cc3274f96&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44349&comment_hash=0bd80f2e5b74766a526dd436ea04447d2896848992aa1710995b172cc3274f96&reaction=dislike'>👎</a>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] perf(deletion-retention): one window pass for repeat predicate [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44349:
URL: https://github.com/apache/superset/pull/44349#issuecomment-5965738878
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**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 <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org