viirya commented on pull request #33650: URL: https://github.com/apache/spark/pull/33650#issuecomment-902867028
> @viirya If you look at the file scan (e.g. `ParquetScan`, https://github.com/apache/spark/blob/master/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/parquet/ParquetScan.scala#L46), you will see that file scan has `partitionFilters: Seq[Expression]`, `dataFilters: Seq[Expression]` and `pushedFilters: Array[Filter]`(contains both partition filter and data filters). The reason that we need `partitionFilters: Seq[Expression]`, `dataFilters: Seq[Expression]` in the format of `Expression` is that we need the `Expression` filters to do partition pruning. `pushFilters` takes the `sources.Filters`, and we will have to construct `sources.Filters` to `Expression` filters. In order to avoid performance regression, we have to special case file source. I will update the PR description to explain the motivation of the changes. I know that you have to push partition filters as catalyst expressions for partition pruning. Currently this is done at `PruneFileSourcePartitions`. Filter pushdown (for `SupportsPushDownFilters`) is done at `V2ScanRelationPushDown`. And you need to have partition filters and data filters separated at `V2ScanRelationPushDown` for aggregate pushdown. The question I have for a while is, looks like you only need to know what partition filters and data filters are at `V2ScanRelationPushDown`. Why you cannot do it at `V2ScanRelationPushDown` simply and use it for aggregate pushdown? The concern for this change is, actually it breaks the API design for `SupportsPushDownFilters`. Now we have data source V2 which doesn't implement SupportsPushDownFilters but it does filter pushdown. For example, if we need to catch data sources that are capable of filter pushdown somewhere, seems we now cannot simply catch `SupportsPushDownFilters` interface anymore. I feel it is important to keep API consistency and design. -- 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]
