[SPARK-58199][SQL] Add FileIndex fullyPushedFilter hint for physical plan#57340
[SPARK-58199][SQL] Add FileIndex fullyPushedFilter hint for physical plan#57340szehon-ho wants to merge 2 commits into
Conversation
| // for removing filters from the row-level FilterExec above the scan. | ||
| val afterScanFilters = fsRelation.location.fullyPushedFilters match { | ||
| case Some(fullyPushedFilters) => filterSet -- fullyPushedFilters | ||
| case None => filterSet -- partitionKeyFilters.filter(_.references.nonEmpty) |
There was a problem hiding this comment.
nit/nonblocker - I wonder if we can always set partition key filters in index when we see it the first time to avoid the fallback; is that possible?
There was a problem hiding this comment.
yes, let me address with the other concern (that we dont call listFiles yet until physical execution..)
03e3446 to
bec1e80
Compare
| * non-matching rows for that column, so the partition filter is not fully pushed and must be | ||
| * omitted. | ||
| * | ||
| * Note these filters are the same predicates that [[listFiles]] prunes with, but not necessarily |
There was a problem hiding this comment.
This statement worries me. Like I don't know if this is any better that having a simple boolean flag to indicate if FileIndex enforces / pushes down all partition predicates.
There was a problem hiding this comment.
I will have to think a bit more.
There was a problem hiding this comment.
discussed offline, let's remove the dataFilter argument for now as it transforms more before listFiles(). Just passing list of Partition filters is more simple.
Also renamed the method
… plan Add an optional fullyPushedFilters(partitionKeyFilters, dataFilters) method to the FileIndex trait and use it in FileSourceStrategy to determine which filters can be removed from the row-level FilterExec above the scan. The default implementation returns partitionKeyFilters, preserving existing behavior. Co-authored-by: Isaac
Simplify the FileIndex hint to return the subset of partition filters that are fully applied for all returned rows, instead of taking dataFilters and returning an arbitrary filter subset. Removes the javadoc about planner expansion/derived expressions since it no longer applies.
78b23ee to
95cc353
Compare
What changes were proposed in this pull request?
This PR adds an optional
fullyPushedFiltershint to theFileIndextrait andteaches
FileSourceStrategyto use it when deciding which filters can be removedfrom the row-level
FilterExecplaced above the scan.FileIndex.fullyPushedFilters: Option[Seq[Expression]]defaults toNone.FileSourceStrategy, the set of post-scan (after-scan) filters is now computed as:FileIndexreturnsSome(fullyPushedFilters), those filters are theonly basis for removing predicates from the
FilterExecabove the scan(
filterSet -- fullyPushedFilters).None, planning keeps the existing default behavior(
filterSet -- partitionKeyFilters.filter(_.references.nonEmpty)).This lets a
FileIndeximplementation that guarantees certain predicates for allreturned rows (e.g. an index that fully prunes on non-partition columns) tell the
planner exactly which filters are safe to drop, instead of relying only on the
default partition-key heuristic.
Note: in DSV2 , the scalar subquery partition filters are kept on FIlterExec for correctness, but they are removed in DSV1 world.
Why are the changes needed?
Today
FileSourceStrategyassumes only partition-key filters are fully satisfied byfile listing, so any other predicate a
FileIndexmay fully guarantee must still bere-evaluated in a
FilterExec. CustomFileIndeximplementations have no way tocommunicate stronger pushdown guarantees to the physical plan. The new hint provides
an opt-in mechanism, with no behavior change for existing indexes (default
None).Does this PR introduce any user-facing change?
No. The new trait method has a default implementation returning
None, whichpreserves the existing planning behavior for all built-in
FileIndeximplementations.How was this patch tested?
Added a unit test in
FileSourceStrategySuite("FileIndex fullyPushedFilters controls final FilterExec removal") that exercises a
test
FileIndexreturningNone,Some(Nil), andSome(<data filter>), and assertsthe resulting
FilterExecpredicates. Ran:All tests passed.
Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 4.8)