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: [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]
