rdblue commented on code in PR #6622:
URL: https://github.com/apache/iceberg/pull/6622#discussion_r1097936851
##########
spark/v3.3/spark/src/main/java/org/apache/iceberg/spark/source/SparkScanBuilder.java:
##########
@@ -158,6 +182,141 @@ public Filter[] pushedFilters() {
return pushedFilters;
}
+ @Override
+ public boolean pushAggregation(Aggregation aggregation) {
+ if (!pushDownAggregate(aggregation)) {
+ return false;
+ }
+
+ AggregateEvaluator aggregateEvaluator;
+ try {
+ List<Expression> aggregates =
+ Arrays.stream(aggregation.aggregateExpressions())
+ .map(agg -> SparkAggregates.convert(agg))
+ .collect(Collectors.toList());
+ aggregateEvaluator = AggregateEvaluator.create(schema, aggregates);
+ } catch (Exception e) {
+ LOG.info("Can't push down aggregates: " + e.getMessage());
+ return false;
+ }
+
+ if
(!metricsModeSupportsAggregatePushDown(aggregateEvaluator.aggregates())) {
+ LOG.info("The MetricsMode doesn't support aggregate push down.");
+ return false;
+ }
+
+ List<ManifestFile> manifests = getSnapshot().allManifests(table.io());
+
+ for (ManifestFile manifest : manifests) {
+ try (ManifestReader<DataFile> reader = ManifestFiles.read(manifest,
table.io())) {
+ for (DataFile dataFile : reader) {
+ aggregateEvaluator.update(dataFile.copy());
+ }
+ } catch (IOException e) {
+ LOG.info("Can't push down aggregates: " + e.getMessage());
+ return false;
+ }
+ }
+
+ Object[] res = aggregateEvaluator.result();
Review Comment:
@huaxingao, I think the fix is to have a flag in the aggregator that can
return whether or not the value is valid. That's what I wanted to use `null`
for here, but you're right that there are cases where the aggregate is value
and that value is null because there are no non-null values.
If we keep track of `isValid` in each aggregator, then the
`AggregateEvaluator` can have a similar method to return whether all aggregates
are valid. The we would just abort the aggregation if any value is not known.
We can also have an override flag for when you want the closest answer, even if
it isn't guaranteed to be correct.
FYI @aokolnychyi
--
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]
