rdblue commented on code in PR #13195:
URL: https://github.com/apache/iceberg/pull/13195#discussion_r2229808504


##########
core/src/test/java/org/apache/iceberg/expressions/TestInclusiveMetricsEvaluatorWithExtract.java:
##########
@@ -91,7 +165,22 @@ public class TestInclusiveMetricsEvaluatorWithExtract {
                       "$['event_id']",
                       Variants.of(INT_MIN_VALUE),
                       "$['str']",
-                      Variants.of("abc")))),
+                      Variants.of("abc"),
+                      "$['event_uuid']",
+                      Variants.ofUUID(VAR_UUID),
+                      "$['event_timestamp_long']",
+                      Variants.of(TS_MIN_VALUE),
+                      "$['event_timestamp_ts_type']",
+                      
Variants.ofIsoTimestamptz(DateTimeUtil.microsToIsoTimestamptz(TS_MIN_VALUE)),
+                      "$['event_timestamp_nano_long']",
+                      Variants.of(TS_NANO_MIN_VALUE),
+                      "$['event_timestamp_nano_ts_type']",
+                      Variants.ofIsoTimestamptzNanos(
+                          
DateTimeUtil.nanosToIsoTimestamptz(TS_NANO_MIN_VALUE)),
+                      "$['event_time_long']",
+                      Variants.of(TIME_MIN_VALUE),
+                      "$['event_time_ts_type']",
+                      
Variants.ofIsoTime(DateTimeUtil.microsToIsoTime(TIME_MIN_VALUE))))),

Review Comment:
   The tests in this file validate expression types, not data types. To add 
type tests, I think this only needs to validate that the expression is passed 
and handled correctly.
   
   To do that, I think you should use a parameterized test with a type, a 
literal, a variant lower bound, and a variant upper bound. The test itself 
should construct the `DataFile` and then call `shouldRead`. I think it should 
be a lot simpler than the current changes.



-- 
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