Handle cases that definitely cannot match for notStartsWith() Predica…

[manirajv06]

…te in StrictMetricsEvaluator

Fixes #13765

[manirajv06]

@nastra @rdblue @kbendick Can you take a look?

[nastra]

@manirajv06 can you fix CI please?

[manirajv06]

@manirajv06 can you fix CI please?

Fixed. Please check.

[manirajv06]

@nastra @rdblue Had a chance to review the changes? Thanks.

[smaheshwar-pltr]

Thanks for the contribution, @manirajv06!

I'm not sure the logic is quite right as is - it seems to claim that if both the lower and upper bounds don't start with the prefix, then all rows don't start with the prefix.

However, consider notStartsWith("b") with bound ["a", "z"] - the logic above would report that all rows don't start with "b", but "b" could very well be a row lying between "a" and "z".

I think that the TODO here before was referring to the case where the prefix lies outside the bounds, in which case no row can match. There are similar semantics in the inclusive metric evaluation of the startsWith predicate, which I think it symmetric to this case:

iceberg/api/src/main/java/org/apache/iceberg/expressions/InclusiveMetricsEvaluator.java

Line 388 in 7b9ea75

public <T> Boolean startsWith(Bound<T> term, Literal<T> lit) {

[manirajv06]

@smaheshwar-pltr Thanks for taking a look. Sorry for the delay.

My understanding is, Unlike Inclusive, Strict would answer only if it is able to find the values deterministically. Thats the reason for returning ROWS_MIGHT_NOT_MATCH in the initial cut as that would be more accurate from defensive stand point of view with the comment to catch the case that cannot match for sure later. I have made the changes to catch the cases that cannot match for sure using the prefix.

However, consider notStartsWith("b") with bound ["a", "z"] - the logic above would report that all rows don't start with "b", but "b" could very well be a row lying between "a" and "z".

Yes, this should return true in Inclusive, but should not be in Strict.

@rdblue Can you please check? Is my understanding correct?

[github-actions]

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the [email protected] list. Thank you for your contributions.

[manirajv06]

recheck

[github-actions]

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the [email protected] list. Thank you for your contributions.

[github-actions]

This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.