peter-toth commented on PR #58145: URL: https://github.com/apache/spark/pull/58145#issuecomment-6640685211
Thanks @aokolnychyi. On 3, I agree, and I think that should come first. Since SPARK-58315 (in 4.3.0), the pushed filters that are re-added for a scan with `reflectsFullyPushedDownFilters() == false` are executed. That happens without CBO too, where nothing reads their estimate. Dropping them and the inferred ones from the executed plan fixes that today. The simple solution depends on it too. Is there a ticket for it? On 1, the migration argument makes sense to me. Why do the pushed and the remaining predicates need to be estimated together, though? Is it about predicates on the same column? `FilterEstimation` updates the column stats from one conjunct to the next, so `a > 5 AND a < 10` gets the intersection of the two ranges. If a connector adjusts only the row count for a pushed `a > 5`, a remaining `a < 10` is estimated on the old range. A connector that also adjusts the column stats wouldn't need this. On 2, how would the stats visitor use the map? An inferred predicate is implied by its sources. So it adds nothing where Spark can estimate the sources, and it is an upper bound where Spark can't. With `[A -> [C], B -> [C, D]]`, when A is estimable and B is not, should C be included? It overlaps A, but A alone doesn't imply it. The expression set keeps C from being inserted twice, but it doesn't decide whether to insert it once. Given these open questions, I'm not against the simple solution that accepts the double counting, once 3 is solved. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
