Conversation
szehon-ho
left a comment
szehon-ho
left a comment
There was a problem hiding this comment.
LGTM. The three inline suggestions are optional style cleanups for clearer Scala. They should be applied together because they change the helper callback type.
Withdrawing my approval: on another reading, we found that retaining both an extracted predicate and its original residual can underestimate cardinality. Sorry I missed this in the initial review.
There was a problem hiding this comment.
Sorry for missing this in my initial review. I found a statistics issue on another reading, so I have withdrawn my approval for now. I had the same finding when trying to do the 'inferred filter' DSV2 API: https://github.com/apache/spark/pull/58145/changes Details below
| extractPushablePredicate( | ||
| filterExpr, | ||
| DataSourceV2Strategy.translateFilterV2) | ||
| .flatMap(translateFilter) | ||
| .foreach(translatedFilters += _) |
There was a problem hiding this comment.
Could we avoid retaining the synthesized predicate as an additional post-scan filter when the original expression is already retained? This applies to both filter APIs.
For example, let P = id = 1 OR (id = 2 AND U) and let the extracted predicate be Q = id = 1 OR id = 2, where U cannot be translated. If the source rejects Q and returns it for reevaluation, residual reconstruction produces Q AND P. Since P already implies Q, that extra condition is redundant.
This can affect statistics: FilterEstimation multiplies the selectivities of the two sides of AND, while estimating OR does not update column statistics. With 10 non-null distinct ID values, no histogram, and U unsupported by the estimator, the estimated selectivity is 19% for P but 3.61% for Q AND P. Cost-based join reordering runs after scan pushdown and before the later boolean cleanup, so the redundant filter can affect join planning.
Could we track the predicates synthesized by partial extraction and omit their returned residuals when the original predicate is still enforced?
There was a problem hiding this comment.
Thanks for catching this — fixed now.
This branch has not been deployed
2 participants
What changes were proposed in this pull request?
Extract necessary predicates when a V2 scan filter cannot be fully translated, for both
SupportsPushDownFiltersandSupportsPushDownV2Filters. Retain the original expression for post-scan filtering.For example,
id = 1 OR (id = 2 AND split(name, ',')[0] = 'fred')can pushid = 1 OR id = 2while Spark evaluates the original condition.Why are the changes needed?
An unsupported expression in one branch currently prevents the entire OR predicate from being pushed down, even when both branches imply a translatable condition. Extracting that condition lets the data source filter rows earlier.
Does this PR introduce any user-facing change?
Yes. Eligible V2 scans push an additional filter; query results are unchanged.
How was this patch tested?
Added regression tests covering both filter APIs, residual filtering, NULL values, unsupported OR branches, NOT, non-deterministic expressions, and fully translatable filters. Added an H2 JDBC test checking the pushed predicate and query results.
Was this patch authored or co-authored using generative AI tooling?
Yes.