You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "bharos (via GitHub)" <gi...@apache.org> on 2026/04/03 23:08:06 UTC
[PR] API: Implement notStartsWith bounds check in StrictMetricsEvaluator [iceberg]
bharos opened a new pull request, #15883:
URL: https://github.com/apache/iceberg/pull/15883
### What
Implements bounds-based evaluation for `notStartsWith` in
`StrictMetricsEvaluator`, replacing the existing TODO with actual logic.
Previously, `notStartsWith` always returned `ROWS_MIGHT_NOT_MATCH`,
which prevented the engine from eliminating the residual predicate even
when file-level column bounds made it provable that no value could start
with the given prefix.
### Changes
- **`StrictMetricsEvaluator.notStartsWith`**: Added checks for nested
columns, all-nulls columns, and lower/upper bound comparisons against
the prefix. Returns `ROWS_MUST_MATCH` when bounds prove the prefix is
entirely outside the value range.
- **`TestStrictMetricsEvaluator`**: Added 8 test methods covering:
all-nulls, bounds above/below/overlapping the prefix, wider ranges,
missing stats, some-nulls with bounds outside prefix, and prefix
longer than bounds.
### How it works
For `NOT STARTS WITH <prefix>`:
- If the lower bound (truncated to `min(prefixLen, boundLen)`) is
strictly greater than the prefix, all values are above the prefix
range → `ROWS_MUST_MATCH`
- If the upper bound (truncated to `min(prefixLen, boundLen)`) is
strictly less than the prefix, all values are below the prefix range
→ `ROWS_MUST_MATCH`
- Otherwise, fall through to `ROWS_MIGHT_NOT_MATCH` (conservative)
This follows the same pattern used by `notEq` and `notIn` in this
class, including the null-handling convention.
Closes #15882
--
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: Implement notStartsWith bounds check in StrictMetricsEvaluator [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer merged PR #15883:
URL: https://github.com/apache/iceberg/pull/15883
--
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: Implement notStartsWith bounds check in StrictMetricsEvaluator [iceberg]
Posted by "bharos (via GitHub)" <gi...@apache.org>.
bharos commented on PR #15883:
URL: https://github.com/apache/iceberg/pull/15883#issuecomment-4246417910
Friendly ping — this has a contributor approval and all checks are green( Flink CI failure should be unrelated)
@nastra @aokolnychyi would either of you be able to review or point me to the right person? Thanks!
--
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: Implement notStartsWith bounds check in StrictMetricsEvaluator [iceberg]
Posted by "bharos (via GitHub)" <gi...@apache.org>.
bharos commented on code in PR #15883:
URL: https://github.com/apache/iceberg/pull/15883#discussion_r3042389910
##########
api/src/main/java/org/apache/iceberg/expressions/StrictMetricsEvaluator.java:
##########
@@ -467,8 +469,36 @@ public <T> Boolean startsWith(BoundReference<T> ref, Literal<T> lit) {
@Override
public <T> Boolean notStartsWith(BoundReference<T> ref, Literal<T> lit) {
- // TODO: Handle cases that definitely cannot match, such as notStartsWith("x") when the bounds
- // are ["a", "b"].
+ int id = ref.fieldId();
+ if (isNestedColumn(id)) {
Review Comment:
Thanks @anoopj , added a nested string field "nested_string_col" in test and a test case
PTAL
--
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: Implement notStartsWith bounds check in StrictMetricsEvaluator [iceberg]
Posted by "anoopj (via GitHub)" <gi...@apache.org>.
anoopj commented on code in PR #15883:
URL: https://github.com/apache/iceberg/pull/15883#discussion_r3042314328
##########
api/src/main/java/org/apache/iceberg/expressions/StrictMetricsEvaluator.java:
##########
@@ -467,8 +469,36 @@ public <T> Boolean startsWith(BoundReference<T> ref, Literal<T> lit) {
@Override
public <T> Boolean notStartsWith(BoundReference<T> ref, Literal<T> lit) {
- // TODO: Handle cases that definitely cannot match, such as notStartsWith("x") when the bounds
- // are ["a", "b"].
+ int id = ref.fieldId();
+ if (isNestedColumn(id)) {
Review Comment:
Consider adding a test for nested column?
--
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: Implement notStartsWith bounds check in StrictMetricsEvaluator [iceberg]
Posted by "anoopj (via GitHub)" <gi...@apache.org>.
anoopj commented on PR #15883:
URL: https://github.com/apache/iceberg/pull/15883#issuecomment-4196161070
The change looks reasonable to me. The only callout is the null handling: if there are null values, the implementation will return ROWS_MUST_MATCH for them as well. This doesn't follow SQL's 3-valued semantics, but is consistent with the current implementation of notEq, so I think the change is reasonable.
Please get this reviewed by a committer.
--
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: Implement notStartsWith bounds check in StrictMetricsEvaluator [iceberg]
Posted by "bharos (via GitHub)" <gi...@apache.org>.
bharos commented on code in PR #15883:
URL: https://github.com/apache/iceberg/pull/15883#discussion_r3133245980
##########
api/src/test/java/org/apache/iceberg/expressions/TestStrictMetricsEvaluator.java:
##########
@@ -684,4 +719,92 @@ SCHEMA, lessThanOrEqual("struct.nested_col_with_stats", INT_MAX_VALUE))
new StrictMetricsEvaluator(SCHEMA, notNull("struct.nested_col_with_stats")).eval(FILE);
assertThat(shouldRead).as("notNull nested column should not match").isFalse();
}
+
Review Comment:
Appreciate the review @RussellSpitzer
Updated the tests to include these cases. PTAL
--
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: Implement notStartsWith bounds check in StrictMetricsEvaluator [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #15883:
URL: https://github.com/apache/iceberg/pull/15883#discussion_r3133014731
##########
api/src/test/java/org/apache/iceberg/expressions/TestStrictMetricsEvaluator.java:
##########
@@ -684,4 +719,92 @@ SCHEMA, lessThanOrEqual("struct.nested_col_with_stats", INT_MAX_VALUE))
new StrictMetricsEvaluator(SCHEMA, notNull("struct.nested_col_with_stats")).eval(FILE);
assertThat(shouldRead).as("notNull nested column should not match").isFalse();
}
+
Review Comment:
Suggestion for two more tests:
1 - Empty prefix
2 - Exact match of bounds
--
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: Implement notStartsWith bounds check in StrictMetricsEvaluator [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on PR #15883:
URL: https://github.com/apache/iceberg/pull/15883#issuecomment-4308090124
Thanks @bharos ! Thanks @anoopj for your review!
--
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