perf(optimizer): convert filter predicate to CNF to push through join

[kevinzwang]

By converting the predicate to Conjunctive Normal Form, we are able to split the predicate into expressions that are isolated to the schemas of an individual side of a join. Then, we can split by conjunctions and push those new expressions through the joins.

For example, consider the schemas:

left = [a: Utf8]
right = [b: Utf8]

If our plan was

Filter
|  by: (col(a) = "FRANCE" & col("b") = "GERMANY") | (col(a) = "GERMANY" & col("b") = "FRANCE")
|
Join
| \
|  Scan(right)
Scan(left)

That filter predicate in CNF is

(col(a) = "FRANCE" | col(a) = "GERMANY") 
  & (col(a) = "FRANCE" | col(b) = "FRANCE") 
  & (col(b) = "GERMANY" | col(a) = "GERMANY") 
  & (col(b) = "GERMANY" | col(b) = "FRANCE")

The first and last expressions in this conjunction only have columns from a single side of the join, so can be pushed down, resulting in this optimized plan:

Filter
|  by: (col(a) = "FRANCE" | col(b) = "FRANCE") & (col(b) = "GERMANY" | col(a) = "GERMANY") 
|
Join
| \
|  Scan(right)
|    pushdowns: Filter { (col(b) = "GERMANY" | col(b) = "FRANCE") }
Scan(left)
|. pushdowns: Filter { (col(a) = "FRANCE" | col(a) = "GERMANY") }

[universalmind303]

is this only applicable to predicates? I'm wondering if this should be part of the SimplifyExprs optimizer pass?

[kevinzwang]

It should be a general simplification yes. Just put the comment here because this part specifically expands the predicate

[codspeed-hq]

CodSpeed Performance Report

Merging #3623 will degrade performances by 33.13%

_{Comparing kevin/better-filter-join (7febe13) with main (07f6b2c)}

Summary

⚡ 2 improvements
❌ 1 regressions
✅ 24 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

	Benchmark	main	kevin/better-filter-join	Change
⚡	test_iter_rows_first_row[100 Small Files]	244 ms	207.4 ms	+17.66%
❌	test_show[100 Small Files]	15.9 ms	23.8 ms	-33.13%
⚡	test_tpch_sql[1-in-memory-native-7]	1,183.3 ms	778.9 ms	+51.91%

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.55%. Comparing base (07f6b2c) to head (7febe13).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3623      +/-   ##
==========================================
- Coverage   77.84%   77.55%   -0.29%     
==========================================
  Files         719      719              
  Lines       88280    88958     +678     
==========================================
+ Hits        68720    68992     +272     
- Misses      19560    19966     +406     

Files with missing lines	Coverage Δ
src/daft-algebra/src/boolean.rs	100.00% <100.00%> (ø)
...al-plan/src/optimization/rules/push_down_filter.rs	97.32% <100.00%> (+1.68%)	⬆️

... and 20 files with indirect coverage changes

[andrewgazelka]

are we worried about expression explosion? like if we have 10 expressions on LHS an 10 on RHS I'd imagine we would sometimes get 100 = $O(n^2)$ expressions.

[andrewgazelka]

can we have more tests with negations?

[andrewgazelka]

in future I suppose could do some rust proptest stuff here to verify they are correct but not needed for this issue imo

[kevinzwang]

are we worried about expression explosion? like if we have 10 expressions on LHS an 10 on RHS I'd imagine we would sometimes get 100 = O ( n 2 ) expressions.

I do want to fix this at some point but most filter predicates should not balloon to significantly large sizes. Will work on more expression simplification stuff in the future to address this