You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "raghavyadav01 (via GitHub)" <gi...@apache.org> on 2026/10/20 05:03:25 UTC
[PR] API: Handle null partition values in ResidualEvaluator startsWith/notStartsWith [iceberg]
raghavyadav01 opened a new pull request, #18590:
URL: https://github.com/apache/iceberg/pull/18590
Closes #18412
`ResidualEvaluator` dereferences the partition value in `startsWith` and `notStartsWith` without a null check, so computing a residual for a partition whose value is null throws `NullPointerException`:
```
java.lang.NullPointerException: Cannot invoke "String.startsWith(String)" because the return value of
"org.apache.iceberg.expressions.BoundReference.eval(org.apache.iceberg.StructLike)" is null
at org.apache.iceberg.expressions.ResidualEvaluator$ResidualVisitor.startsWith(ResidualEvaluator.java:215)
```
`ManifestFilterManager` computes a residual for every live file in a manifest it rewrites, so a delete by row filter fails whenever the manifest also holds a file with a null partition value. Scans hit it too: with `or(isNull(s), startsWith(s, "a"))` the null-partition file survives planning through the `isNull` branch, and `FileScanTask.residual()` then throws.
## Change
Treat a null partition value as not matching, so `startsWith` returns `alwaysFalse()` and `notStartsWith` returns `alwaysTrue()`.
This keeps `ResidualEvaluator` consistent with `Evaluator`, which already evaluates `startsWith` as `evalRes != null && ((String) evalRes).startsWith(...)` and `notStartsWith` as its negation, and it mirrors the guard `in` and `notIn` received in #17014.
## Tests
Added `testStartsWithWithNullStringPartition` and `testNotStartsWithWithNullStringPartition` to `TestResiduals`, following the shape of the existing `testInWithNullStringPartition` / `testNotInWithNullStringPartition` cases. Both fail with `NullPointerException` on main and pass with this change.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API: Handle null partition values in ResidualEvaluator startsWith/notStartsWith [iceberg]
Posted by "zhengxiang378928908-code (via GitHub)" <gi...@apache.org>.
zhengxiang378928908-code commented on PR #18590:
URL: https://github.com/apache/iceberg/pull/18590#issuecomment-6336747294
For reviewers: #18414, opened on October 7 and also addressing #18412, contains the same production change. Both PRs produce the identical `ResidualEvaluator.java` (blob `b03675fa1`). They differ in tests: #18590 changes `TestResiduals`, and #18414 changes `TestResiduals`, `TestTruncatesResiduals` and `TestDeleteFiles`. Could the review be consolidated into one PR?
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API: Handle null partition values in ResidualEvaluator startsWith/notStartsWith [iceberg]
Posted by "raghavyadav01 (via GitHub)" <gi...@apache.org>.
raghavyadav01 commented on PR #18590:
URL: https://github.com/apache/iceberg/pull/18590#issuecomment-6346028074
Thanks @zhengxiang378928908-code for spotting this — you're right, and #18414 should be the one that goes forward.
It predates this by 13 days, it's from @LuciferYang who reported #18412 in the first place, and it carries the wider coverage: `TestResiduals`, `TestTruncatesResiduals` and `TestDeleteFiles` against the two files here. The production change is byte-identical, so there is nothing here that #18414 lacks.
Closing in favour of it. I'll review #18414 instead — apologies for the duplicated review effort, I checked the issue for people claiming it but not for an existing PR referencing it.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API: Handle null partition values in ResidualEvaluator startsWith/notStartsWith [iceberg]
Posted by "raghavyadav01 (via GitHub)" <gi...@apache.org>.
raghavyadav01 closed pull request #18590: API: Handle null partition values in ResidualEvaluator startsWith/notStartsWith
URL: https://github.com/apache/iceberg/pull/18590
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org