amogh-jahagirdar commented on code in PR #9457:
URL: https://github.com/apache/iceberg/pull/9457#discussion_r1451098884
##########
api/src/main/java/org/apache/iceberg/expressions/AggregateEvaluator.java:
##########
@@ -83,6 +83,18 @@ public void update(DataFile file) {
}
}
+ public void update(DataFile file, StructLike struct) {
Review Comment:
Is this new API needed? Why not update the existing API to have the `else
if` case if it's a partition column?
##########
spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/sql/TestAggregatePushDown.java:
##########
@@ -441,6 +441,40 @@ private void testAggregatePushDownWithFilter(boolean
partitionFilerOnly) {
assertEquals("expected and actual should equal", expected, actual);
}
+ @TestTemplate
+ public void testAggregatePushdownOnPartitionColumn() {
Review Comment:
This test still passes for me locally even without any of the code changes.
Is there a stronger test we could use for pushdown on partition columns that
would fail prior to the code changes?
Based on the description this issue is specific to aggregation pushdown on
partition columns when it is a a Hive table being registered as an Iceberg
table so the test should reflect that I think.
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
