Clean-up the planner logic for moving join predicates from filters in sql/planner.rs which is moved to optimizer rule reduce_cross_join.
- Lenguaje dominante
- Rust
- Estrellas
- 9.3k
- Forks
- 2.4k
- Merge medio
- 3 d 11 h
- PR fusionados (30 d)
- 360
Descripción
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_
Guía de contribución
Línea de trabajo
Comience comparando la lógica del planner en sql/planner.rs con la regla del optimizador reduce_cross_join y el rewrite RewriteDisjunctivePredicate, usando PR #3482 y su review como contexto. Determine qué áreas superpuestas deberían consolidarse y, a continuación, verifique el resultado con tests que cubran predicados OR más complejos y consultas que involucren más de dos tablas.
Escrito por el modelo de indexación a partir del texto del issue.
Evaluación
- Stack tecnológico
- rust
- Área
- databases
- Tipo de issue
- Refactorización
- Dificultad
- 4/5
- Tiempo estimado
- 3-5 días
- Estado de actividad
- Estancado
- Claridad
- Necesita aclaración
- Aptitud para principiantes
- 30/100