feat: implement native native existence join support - #4587
Conversation
comphead
left a comment
There was a problem hiding this comment.
Thanks @coderfender this would also need to be tested by sql tests trying to cover as much edge cases as possible. Mark joins are pretty recently added in DF and might require extra testing
|
Absolutely agreed! Thank you for the review @comphead . |
|
Mixed results with benches |
3ebf096 to
d3d400a
Compare
|
Thanks @coderfender. The posted benchmarks show that this would be a performance regression compared to Spark? Did performance improve since those were posted? |
|
Apparently sooner or later we would need to use DF mark joins, they can be disabled by default and we can improve them in background |
|
@andygrove thank you for the comment, Yes the bechmarks are indeed different (with atleast 2x speedup) |
4013717 to
6fc8b32
Compare
|
nice PR in DF in terms of performance apache/datafusion#23870 |
Mapping Spark's Before this can be reviewed properly, though, a few things. The description is empty Every section of the template is still a placeholder: no rationale, no summary of changes, no testing notes. For a change that introduces a new join type mapping across three join operators, the reviewer needs to know what you verified. In particular I would want the description to answer the semantic question below, because a reader cannot get that from the diff. Is Spark's
A short section in the description walking through those, ideally with a DataFusion source link, would make this reviewable.
Benchmark results
One CI note There is a failing check on this PR. Worth looking at before the next round of review. |
Which issue does this PR close?
Closes #3881 .
Rationale for this change
What changes are included in this PR?
How are these changes tested?