yiyi99 commented on issue #25258:
URL: https://github.com/apache/datafusion/issues/25258#issuecomment-5660034658

   > [@yiyi99](https://github.com/yiyi99) if you want to take a look - 
[#25273](https://github.com/apache/datafusion/pull/25273)
   
   Thanks!I tested against this branch (eba49b2, via a git dependency), with 
expected results derived from PostgreSQL semantics (using the same emp data as 
in the issue): 14/14 passed. All previously broken unaliased forms now match 
PostgreSQL behavior, and the previously working aliased forms remain unaffected.
   
   | form | result |
   | --- | --- |
   | > >= < <= × ANY (subquery id <= 3) |  5 / 6 / 2 / 3 rows |
   | > >= < <= × ALL (subquery id <= 2) |  4 / 5 / 0 / 1 rows |
   | = ANY / <> ALL |  2 / 4 rows |
   | nested > ANY (… WHERE id IN (SELECT id FROM emp …)) | PASS |
   | IN / scalar subquery, same unaliased table | PASS |
   | aliased-outer regression check | PASS |
   
   Two review suggestions:
   1. Test surface vs. reported surface: the SLT covers > ANY, > ALL, = ANY, 
but the issue covers all four ordered operators. Since they all route through 
the fixed shared path, I'd suggest adding the remaining variants (>= ANY, < 
ANY, <= ANY, >= ALL, < ALL, <= ALL, <> ALL) and a nested form to the SLT — my 
run above confirms they pass on this branch, so it's just locking them in.
   2. API change note: find_join_exprs is public and its contract changed (it 
no longer strips outer references). Engines embedding DataFusion that call it 
directly would silently get different filters back — might be worth a doc 
comment / changelog entry.
   
   Also glad to see #25276 filed for the mark-join plan shape. Once this lands 
in a release we'll drop our alias-injection workaround downstream. Thanks again!
   
   


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