JeonDaehong commented on code in PR #18673:
URL: https://github.com/apache/iceberg/pull/18673#discussion_r4430843945
##########
api/src/main/java/org/apache/iceberg/expressions/ResidualEvaluator.java:
##########
@@ -165,13 +165,15 @@ public <T> Expression notNaN(BoundReference<T> ref) {
@Override
public <T> Expression lt(BoundReference<T> ref, Literal<T> lit) {
Comparator<T> cmp = lit.comparator();
- return (cmp.compare(ref.eval(struct), lit.value()) < 0) ? alwaysTrue() :
alwaysFalse();
+ T value = ref.eval(struct);
+ return value != null && cmp.compare(value, lit.value()) < 0 ?
alwaysTrue() : alwaysFalse();
Review Comment:
`not(greaterThanOrEqual("d", "2026-10-27"))` still gives `true` for a null
partition with this patch, since `not` is evaluated as `!gtEq` here and nothing
calls `rewriteNot` first. `ManifestFilterManager` passes the delete expression
straight to `ResidualEvaluator`, so from the code
`deleteFromRowFilter(not(greaterThanOrEqual(...)))` should still drop the null
file in the shared manifest case. the metrics evaluators rewrite `not` first
and turn this into `lt`. would rewriting it here too make sense?
##########
api/src/test/java/org/apache/iceberg/transforms/TestResiduals.java:
##########
@@ -49,6 +49,25 @@
import org.junit.jupiter.api.Test;
public class TestResiduals {
+ @Test
+ void lessThanKeepsNullDatePartition() {
+ Schema schema = new Schema(Types.NestedField.optional(1, "d",
Types.DateType.get()));
+ PartitionSpec spec =
PartitionSpec.builderFor(schema).identity("d").build();
+ ResidualEvaluator evaluator = ResidualEvaluator.of(spec, lessThan("d",
"2026-10-27"), true);
+
+ assertThat(evaluator.residualFor(Row.of((Object)
null))).isEqualTo(alwaysFalse());
Review Comment:
I also checked day(ts), month(ts) and truncate on int and string. they go
through the strict projection into the same `lt`/`ltEq`, and all give false for
a null partition with this patch (true before). a day(ts) case here would keep
that covered.
--
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]
