Clean-up the planner logic for moving join predicates from filters in sql/planner.rs which is moved to optimizer rule reduce_cross_join.
- Lingua principale
- Rust
- Stelle
- 9.3k
- Fork
- 2.4k
- Merge medio
- 3g 11h
- PR unite (30g)
- 362
Descrizione
PR#3482
Thank you @DhamoPS -- I reviewed the tests carefully and they look very good 👌 I think if we add a few more showing this PR works with some more complicated `OR` predicates and with more than 2 tables it would be ready to merge.
Thank you @avantgardnerio for your thorough review
I went over the code a bit, and it also looks quite reasonable. I left a few comments but they are all stylistic.
I can't help feeling after this there are three somewhat redundant overlapping areas of the code
1. This one
2. the planner logic identified by @xudong963 in https://github.com/apache/arrow-datafusion/blob/master/datafusion/sql/src/planner.rs#L630-L681
3. The rewrite in RewriteDisjunctivePredicate
Maybe we can work as a follow on to consolidate / remove the redundancy.
All in all very nice work 👍
_Originally posted by @alamb in https://github.com/apache/arrow-datafusion/pull/3482#pullrequestreview-1109803424_
Guida per i contributori
Apri la guida per i contributori
Direzione di ricerca
Iniziate confrontando la logica del planner in sql/planner.rs con la regola dell’optimizer reduce_cross_join e il rewrite RewriteDisjunctivePredicate, usando PR #3482 e la relativa review come contesto. Determinate quali aree sovrapposte dovrebbero essere consolidate, quindi verificate il risultato con test che coprano predicati OR più complessi e query che coinvolgano più di due tabelle.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Valutazione
- Stack tecnologico
- rust
- Ambito
- databases
- Tipo di issue
- Refactoring
- Difficoltà
- 4/5
- Tempo stimato
- 3-5 giorni
- Stato di attività
- Ferma
- Chiarezza
- Da chiarire
- Idoneità per principianti
- 30/100