viirya opened a new pull request, #24361:
URL: https://github.com/apache/datafusion/pull/24361

   ## Which issue does this PR close?
   
   - Closes #24360.
   
   ## Rationale for this change
   
   `PiecewiseMergeJoinExec::compute_properties` passed the join's `on` pair to 
`join_equivalence_properties` as if it were an equijoin key. For an `INNER` 
join that registers `left_on == right_on` as an output equivalence — but PWMJ's 
`on` is a **range** predicate (`l.v < r.v`), not equality, so the equivalence 
is false. It let the optimizer treat a sort on the left key as also sorting the 
right key and drop a required `ORDER BY`, returning wrongly ordered rows.
   
   ```sql
   set datafusion.optimizer.enable_piecewise_merge_join = true;
   create table l(v int) as values (1),(2),(3),(5),(8);
   create table r(v int) as values (4),(6),(9),(2);
   select l.v, r.v from l join r on l.v < r.v where l.v = 2 order by r.v;
   -- PWMJ: 2,9 / 2,6 / 2,4   (wrong order)
   -- NLJ:  2,4 / 2,6 / 2,9   (correct)
   ```
   The plans differ: PWMJ sorts only on `l.v` (`SortExec: expr=[v@0 ASC]`); NLJ 
sorts on both (`[v@0 ASC, v@1 ASC]`).
   
   ## What changes are included in this PR?
   
   - `compute_properties` no longer passes the range `on` pair to 
`join_equivalence_properties` (a range join adds no column equivalences). The 
now-unused `join_on` parameter is dropped.
   - Existing `pwmj.slt` plans/results updated to the corrected (fully sorted) 
output — they previously encoded the wrong ordering — and a regression test is 
added.
   
   ## Are these changes tested?
   
   Yes.
   
   - New regression test in `pwmj.slt` (INNER `l.v < r.v`, `WHERE l.v = 2`, 
`ORDER BY r.v`) asserting the correctly ordered result.
   - The existing `pwmj.slt` cases that asserted the wrong ordering / a 
single-column sort plan are updated to the correct values (verified against 
`NestedLoopJoin`).
   - `piecewise` unit tests and `joins.slt` still pass. Only `INNER` is 
affected.
   
   ## Are there any user-facing changes?
   
   `INNER` range joins via `PiecewiseMergeJoin` (behind 
`enable_piecewise_merge_join`, default off) no longer drop a required sort, 
matching `NestedLoopJoin`. No API changes.
   


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