manirajv06 commented on code in PR #13195:
URL: https://github.com/apache/iceberg/pull/13195#discussion_r2143223534
##########
core/src/test/java/org/apache/iceberg/expressions/TestInclusiveMetricsEvaluatorWithExtract.java:
##########
@@ -683,4 +784,1649 @@ public void testIntegerNotIn() {
.as("Should read: id above upper bound (85 > 79, 86 > 79)")
.isTrue();
}
+
+ @Test
+ public void testTimestampLt() {
+ assertThat(
+ shouldRead(
+ lessThan(
+ extract("variant", "$.event_timestamp_long", "long"),
L_TS_MIN_VALUE_MINUS_10)))
+ .as("Should not read: timestamp range below lower bound (20 > 30)")
+ .isFalse();
+ assertThat(
+ shouldRead(
+ lessThan(extract("variant", "$.event_timestamp_long", "long"),
TS_MIN_VALUE)))
+ .as("Should read: one possible timestamp")
+ .isFalse();
+ assertThat(
+ shouldRead(
+ lessThan(extract("variant", "$.event_timestamp_long", "long"),
TS_MAX_VALUE)))
+ .as("Should read: may possible timestamp")
+ .isTrue();
+ assertThat(
+ shouldRead(
+ lessThan(
+ extract(
+ "variant",
+ "$.event_timestamp_long",
+ Types.TimestampType.withZone().toString()),
+ TS_COMMON_MIN_VALUE_MINUS_10.toString())))
Review Comment:
yes, compares in long.
actual lower bound value is in long/int64, but we would like to compare
against string input. earlier `castTo` used to return null which lead to
ROW_MIGHT_MATCH result. With this change, `castTo` returns the actual lower
bound value which lead to do compare with string input.
--
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
