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: [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]
