juliuszsompolski opened a new pull request, #57226:
URL: https://github.com/apache/spark/pull/57226

   ### What changes were proposed in this pull request?
   
   `SQLLastAttemptAccumulator.extractStageRDDScopes` walks a Dataset's 
`executedPlan` to find the
   `RDDOperationScope` ids of the stages that make up its execution, so that 
`SQLLastAttemptAccumulator`
   can attribute "last attempt" metric updates to that execution. Its 
`BaseSubqueryExec` match handles
   `SubqueryExec`, `SubqueryBroadcastExec` and `SubqueryAdaptiveBroadcastExec`, 
and then bails out on a
   catch-all `case p =>` that records a `bailOutReason`. When extraction bails 
out,
   `lastAttemptValueForQueryExecution` / `lastAttemptValueForDataset` return 
`None`.
   
   `ReusedSubqueryExec` was not enumerated, so any plan containing a reused 
subquery hit the catch-all
   and caused `lastAttemptValueForDataset` / 
`lastAttemptValueForQueryExecution` to return `None`.
   Subquery reuse is enabled by default (`spark.sql.execution.reuseSubquery`), 
and a
   `ReusedSubqueryExec` is produced whenever the same subquery appears more 
than once in a plan --
   including when predicate pushdown duplicates a subquery into a scan's data 
filters.
   
   This PR adds a `ReusedSubqueryExec` case that recurses into its `child`, 
mirroring the existing
   `ReusedExchangeExec` handling. `ReusedSubqueryExec` is a `LeafExecNode` 
whose `child` is a field
   rather than a plan child, so the normal plan traversal never descends into 
it; recursing collects
   the reused subquery's scopes. If the original `SubqueryExec` is still 
present elsewhere in the plan,
   its scopes are collected twice, but that is harmless: 
`lastAttemptValueForRDDScopes` deduplicates by
   scope (it reduces the collected scope ids to a set and aggregates one RDD 
per scope).
   
   ### Why are the changes needed?
   
   Without this, `lastAttemptValueForDataset` / 
`lastAttemptValueForQueryExecution` silently return
   `None` for any query whose physical plan contains a `ReusedSubqueryExec`, 
even though the metric was
   tracked correctly. Because subquery reuse is on by default, this affects 
common queries -- e.g. the
   same scalar/`IN` subquery referenced twice, or a subquery duplicated into a 
`FileScan`'s data filters
   by predicate pushdown.
   
   ### Does this PR introduce _any_ user-facing change?
   
   No.
   
   ### How was this patch tested?
   
   Added a test to `SQLLastAttemptMetricPlanShapesSuite` whose physical plan 
contains a
   `ReusedSubqueryExec`: a scalar subquery with a nested `(SELECT MIN(id) FROM 
range(5))` predicate that
   gets pushed into the outer subquery's scan data filters and duplicated, so 
subquery reuse rewrites one
   copy to a `ReusedSubqueryExec`. The test asserts both the metric value and 
(via
   `testPhysicalPlanShape`) that a `ReusedSubqueryExec` is actually present in 
the plan -- with a
   fallback for the forced-AQE-replan variants, which bypass subquery reuse and 
keep the duplicated
   subquery as two separate `SubqueryExec` nodes (this mirrors the existing 
`exchange - Shuffle` test's
   `ReusedExchangeExec` fallback). Before the fix, `lastAttemptValueForDataset` 
returned `None` for this
   plan (assertion `None did not equal Some(300)`); after the fix it returns 
the expected value. The
   test runs across the suite's existing `useAQE` x `failureMode` x 
`aqeReplans` matrix (9 variants).
   
   ### Was this patch authored or co-authored using generative AI tooling?
   
   Generated-by: Claude Code (Opus 4.8)


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

Reply via email to