nastra commented on code in PR #9176: URL: https://github.com/apache/iceberg/pull/9176#discussion_r1464437182
########## spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/sql/TestAggregatePushDown.java: ########## @@ -249,6 +250,78 @@ public void testAggregateNotPushDownIfOneCantPushDown() { assertEquals("expected and actual should equal", expected, actual); } + @Test + public void testAggregationPushdownStructInteger() { + testAggregationPushdownStruct( + 2L, + 3L, + 2L, + "(id BIGINT, struct_with_int STRUCT<c1:BIGINT>)", + "struct_with_int.c1", + "(1, named_struct(\"c1\", NULL))", + "(2, named_struct(\"c1\", 2))", + "(3, named_struct(\"c1\", 3))"); + } + + @Test + public void testAggregationPushdownNestedStruct() { + testAggregationPushdownStruct( + 2L, + 3L, + 2L, + "(id BIGINT, struct_with_int STRUCT<c1:STRUCT<c2:STRUCT<c3:STRUCT<c4:BIGINT>>>>)", + "struct_with_int.c1.c2.c3.c4", + "(1, named_struct(\"c1\", named_struct(\"c2\", named_struct(\"c3\", named_struct(\"c4\", NULL)))))", + "(2, named_struct(\"c1\", named_struct(\"c2\", named_struct(\"c3\", named_struct(\"c4\", 2)))))", + "(3, named_struct(\"c1\", named_struct(\"c2\", named_struct(\"c3\", named_struct(\"c4\", 3)))))"); + } + + @Test + public void testAggregationPushdownStructTimestamp() { + long timestamp = System.currentTimeMillis(); + long futureTimestamp = timestamp + 5000; + Timestamp expectedMax = new Timestamp(futureTimestamp / 1000 * 1000); + Timestamp expectedMin = new Timestamp(1000 * (timestamp / 1000)); + testAggregationPushdownStruct( + 2L, + expectedMax, + expectedMin, + "(id BIGINT, struct_with_ts STRUCT<c1:TIMESTAMP>)", + "struct_with_ts.c1", + "(1, named_struct(\"c1\", NULL))", + String.format( + "(2, named_struct(\"c1\", CAST(from_unixtime(%d/1000) AS TIMESTAMP)))", timestamp), + String.format( + "(3, named_struct(\"c1\", CAST(from_unixtime(%d/1000) AS TIMESTAMP)))", + timestamp + 5000)); + } + + private void testAggregationPushdownStruct( + Object expectedCount, + Object expectedMax, + Object expectedMin, + String schema, + String aggField, + String... rows) { + sql("CREATE TABLE %s %s USING iceberg", tableName, schema); + sql("INSERT INTO TABLE %s VALUES %s", tableName, String.join(",", rows)); + List<Object[]> actual = + sql("SELECT COUNT(%s), MAX(%s), MIN(%s) FROM %s", aggField, aggField, aggField, tableName); + Object actualCount = actual.get(0)[0]; + Object actualMax = actual.get(0)[1]; + Object actualMin = actual.get(0)[2]; + Assertions.assertThat(actualCount) + .withFailMessage("Expected and actual count should equal") + .isEqualTo(expectedCount); + Assertions.assertThat(actualMax) + .withFailMessage("Expected and actual max should equal") + .isEqualTo(expectedMax); + Assertions.assertThat(actualMin) + .withFailMessage("Expected and actual min should equal") Review Comment: it's better to use `.as()` instead of `withFailMessage()` as otherwise we'd be losing the entire context why the assertion failed and what values actual/expected had -- 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