rdblue commented on code in PR #6981:
URL: https://github.com/apache/iceberg/pull/6981#discussion_r1155365348
##########
spark/v3.3/spark/src/main/java/org/apache/iceberg/spark/source/SparkScanBuilder.java:
##########
@@ -314,6 +306,66 @@ private boolean
metricsModeSupportsAggregatePushDown(List<BoundAggregate<?, ?>>
return true;
}
+ private boolean pushAggregates(List<BoundAggregate<?, ?>> expressions,
Aggregation aggregation) {
+ for (AggregateFunc aggregateFunc : aggregation.aggregateExpressions()) {
+ try {
+ Expression expr = SparkAggregates.convert(aggregateFunc);
+ if (expr != null) {
+ Expression bound = Binder.bind(schema.asStruct(), expr,
caseSensitive);
+ expressions.add((BoundAggregate<?, ?>) bound);
+ } else {
+ LOG.info(
+ "Skipping aggregate pushdown: AggregateFunc {} can't be
converted to iceberg expression",
+ aggregateFunc);
+ return false;
+ }
+ } catch (Exception e) {
+ LOG.info("Skipping aggregate pushdown: Bind failed for AggregateFunc
{}", aggregateFunc, e);
+ return false;
+ }
+ }
+
+ return true;
+ }
+
+ private boolean pushGroupBy(
+ List<BoundGroupBy<?, ?>> groupByExpressions, Aggregation aggregation) {
+ for (org.apache.spark.sql.connector.expressions.Expression groupby :
+ aggregation.groupByExpressions()) {
+ try {
+ if (groupby instanceof NamedReference) {
+ Expression groupByExpr =
+ Expressions.groupBy(SparkUtil.toColumnName((NamedReference)
groupby));
+
+ if (groupByExpr != null) {
+ Expression bound = Binder.bind(schema.asStruct(), groupByExpr,
caseSensitive);
+ if (!ExpressionUtil.isPartitionCol((BoundGroupBy) bound, table)) {
+ return false;
+ }
+
+ groupByExpressions.add((BoundGroupBy) bound);
+ } else {
+ LOG.info(
+ "Skipping aggregate pushdown: groupby {} can't be converted to
iceberg expression",
+ groupby);
+ return false;
+ }
+ } else {
+ LOG.info(
+ "Skipping aggregate pushdown: groupby {} can't be converted to
iceberg expression",
+ groupby);
+ return false;
+ }
+
+ } catch (Exception e) {
Review Comment:
This is too broad. Can you make it more specific? We generally don't like to
catch `Exception`.
--
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]
