aokolnychyi commented on PR #58145: URL: https://github.com/apache/spark/pull/58145#issuecomment-6639684899
@peter-toth, replies in line. 1. The reason `reflectsFullyPushedDownFilters` exists is to allow connectors to opt-in into adjustment of the stats on the Spark side. Without that, it would be challenging to migrate DSv1 data sources that currently rely on Spark adjustments. These adjustments can be non-trivial and moving all of them into the connector would force the connectors to become more complex. Plus, only Spark keeps track of the predicates that are not pushed. We may want to combine all of the predicates together when computing stats. 2. If the optimizer rewrites the original condition so that we can no longer adjust the estimate, it is not going to be the end of the world. Plus, most of the expression simplification must happen **_before_** we create a scan. It will be possible to represent the case with multiple source conditions as the map. Say we have predicates A and B and we infer C from A + B and we infer D from B. Then the map would be [A -> [C], B -> [C, D]]. While transforming the original condition we can use an expression set to avoid inserting C multiple times. 3. Both fully pushed down predicates as well as inferred predicates **_must_** be discarded from the execution plan. They are only needed in the optimized plan for the filter adjustment. It is just a missing functionality that Yan is supposed to fix. My meta point here is that we either go for a simple solution and accept the double counting or do the right thing. The current implementation in this PR is neither simple nor correct. I feel like capturing the map gets us close to being correct but doing a simple solution is not that bad either. The most common case would be converting a predicate on a data column into a partition predicate by understanding generated expressions or hidden partitioning. Unless you have NDVs for the source column and the original predicate is selective, this double counting would never be an issue. What do you think, @peter-toth @szehon-ho? -- 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]
