refactor: simplify expressions #3281
Conversation
ion-elgreco
requested review from
wjones127,
roeap,
rtyler and
hntd187
as code owners
ion-elgreco
force-pushed
the
refactor--simplify-expressions-before-passing-them-around
branch
from
d484c5a to
49f32db
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3281 +/- ##
==========================================
- Coverage 72.14% 72.13% -0.01%
==========================================
Files 143 143
Lines 45624 45641 +17
Branches 45624 45641 +17
==========================================
+ Hits 32915 32923 +8
- Misses 10621 10625 +4
- Partials 2088 2093 +5 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Ion Koutsouris <15728914+ion-elgreco@users.noreply.github.com>
ion-elgreco
force-pushed
the
refactor--simplify-expressions-before-passing-them-around
branch
from
49f32db to
7a51022
Compare
| use tracing::field::debug; | ||
| use tracing::log::*; |
There was a problem hiding this comment.
debug! comes in on the second line, I'm guessing this was an IDE trying to be clever. I'll remove this in a later change so as not to block this.
| @@ -844,11 +848,32 @@ async fn execute( | |||
| streaming, | |||
| ) | |||
| .await? | |||
| } | |||
| .map(|e| { | |||
| // simplify the expression so we have | |||
There was a problem hiding this comment.
So we have what? Quite a cliff-hanger of a comment! 😆 I hope you'll be able to expand on the commentary here in a later change
There was a problem hiding this comment.
😆 I guess I finished the comment in my mind only haha
I'll push a follow-up later
ion-elgreco
deleted the
refactor--simplify-expressions-before-passing-them-around
branch
Description
During merge we should simplify the expression before creating the commit predicate. Additionally, inside the DeltaScan we also need to simplify the expression to improve the pruning.
This doesn't fully fix it but discussing the issue here: apache/datafusion#14944