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

   ### What changes were proposed in this pull request?
   
   Backport of #57347 (SPARK-57956) to `branch-4.2`.
   
   `LogicalQueryStage.maxRows` now uses the stage's `rowCount` only when the 
stage is materialized and its statistics are runtime statistics. Otherwise it 
falls back to `logicalPlan.maxRows`, so a cost estimate is never used as a hard 
upper bound by `EliminateLimits` during AQE re-optimization.
   
   The `LogicalQueryStage` change applies cleanly. `AdaptiveQueryExecSuite` 
needed a manual resolution of the import block only, because `master` imports 
names used by later tests that are not on `branch-4.2`. The added test is 
identical to the one in #57347.
   
   ### Why are the changes needed?
   
   SPARK-57956 was merged into `master`, `branch-4.x` and `branch-4.3`, but not 
into `branch-4.2` or `branch-4.1`, which still have the old `maxRows`:
   
   ```scala
   override def maxRows: Option[Long] = 
stats.rowCount.map(_.min(Long.MaxValue).toLong)
   ```
   
   #59434 reports a wrong result on 4.1.3 from the same cause: with AQE on, a 
nested `LIMIT ... OFFSET` over a `LEFT JOIN` sometimes returns more rows than 
its outer `LIMIT`.
   
   ```sql
   SELECT x FROM (
     SELECT x FROM (
       SELECT p.id AS x FROM p LEFT JOIN (
         SELECT p.id AS anchor FROM p INNER JOIN k ON p.id = k.a INNER JOIN p 
AS q ON k.b = q.id
       ) o ON p.id = o.anchor
       LIMIT 6 OFFSET 2
     ) LIMIT 3 OFFSET 1
   )
   ```
   
   With the tables from #59434 (RDD-backed, `local[1]`, 
`spark.sql.shuffle.partitions=2`), running this query 200 times on `branch-4.2`:
   
   | | AQE on | AQE off |
   |---|---|---|
   | Before this PR | 3 rows: 181 runs, **5 rows: 19 runs** | 3 rows: 200 runs |
   | After this PR | 3 rows: 200 runs | 3 rows: 200 runs |
   
   ### Does this PR introduce _any_ user-facing change?
   
   Yes. It fixes a correctness bug where, with AQE enabled, a query with a 
LIMIT over certain plans could return more rows than the LIMIT.
   
   ### How was this patch tested?
   
   - The new test `SPARK-57956: unmaterialized query stage exposes structural 
maxRows, not the estimate` passes.
   - The existing test `SPARK-36424: Support eliminate limits in AQE Optimizer` 
still passes.
   - The #59434 query above, run 200 times, as shown in the table.
   
   ### Was this patch authored or co-authored using generative AI tooling?
   
   Generated-by: Claude Code (Claude Opus 5.5)
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


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