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

Reply via email to