Flink: Support aggregate push down for Iceberg table source in batch - #17946
Flink: Support aggregate push down for Iceberg table source in batch#17946Guosmilesmile wants to merge 5 commits into
Conversation
0cf326f to
2c67c2b
Compare
|
|
||
| this.filters = expressions; | ||
| return Result.of(acceptedFilters, flinkFilters); | ||
| return Result.of(acceptedFilters, remainingFilters); |
There was a problem hiding this comment.
applyFilters now drops partition-aligned filters from the remaining set for every query, and RowDataFileScanTaskReader grew a wider read schema to compensate - both are always on regardless of the new flag, and neither is in the summary. Split the filter push-down change into its own PR so it can be reviewed and released independently of the opt-in aggregate feature.
There was a problem hiding this comment.
I’m planning to keep this as is for now and see what everyone thinks about the overall direction. If the PR is considered large, we can split it up later.
2c67c2b to
86dcdbd
Compare
51b8ec8 to
4d9c285
Compare
| return iter; | ||
| } | ||
|
|
||
| private Schema filterReadSchema(Schema projected, List<Expression> filters) { |
There was a problem hiding this comment.
In the table source path the projection can only be missing a filter column when applyFilters already proved that filter selects whole partitions, and those files all have a TRUE residual, so the row filter can never drop a row. Consider keeping such filters out of the row filter so the extra columns are not read at all; follow-up, not a blocker.
There was a problem hiding this comment.
I'd suggest keeping the current logic as-is for now and handling it in a follow-up.
| public static final ConfigOption<Boolean> CASE_SENSITIVE_OPTION = | ||
| ConfigOptions.key(PREFIX + CASE_SENSITIVE).booleanType().defaultValue(false); | ||
|
|
||
| public static final String AGGREGATE_PUSH_DOWN_ENABLED = "aggregate-push-down-enabled"; |
There was a problem hiding this comment.
This is the only read option here declared without a connector.iceberg. ConfigOption, so the Flink config spelling that works for every neighbouring read option does nothing for this one. Was the table.exec.iceberg namespace a deliberate choice here?
There was a problem hiding this comment.
Yes, because this is more of an optimization at the execution layer.
a77babe to
18e6ae5
Compare
Summary
Adds aggregate push down to the Flink Iceberg source (
IcebergTableSource). For batch queries that aggregate the whole table withoutGROUP BYorLIMIT,COUNT(*),COUNT(col),MAX(col)andMIN(col)can now be answered from file-level metrics alone, without reading any data files.The feature is opt-in and disabled by default via a new Flink configuration:
Supported scenarios
GROUP BYorLIMIT.COUNT(*),COUNT(col),MAX(col),MIN(col).When push down is skipped (falls back to a regular scan)
GROUP BYorLIMITpresent.TRUEresidual).MIN/MAX, or truncated bounds on string/binary columns).Alignment with Spark
Spark already supports aggregate push down via
spark.sql.iceberg.aggregate-push-down.enabled. This change mirrors that design:AggregatePushDownUtil.metricsModeSupportsAggregatePushDown), now used by both Spark 4.1 and Flink to avoid duplicated logic.AggregateEvaluator.update) follows the same pattern as Spark'spushAggregation.