fix(expr): prune In predicates that straddle metrics bounds

[M-Tesla]

Which issue does this PR close?

• Closes In predicate is not pruned when its literals straddle the bounds (InclusiveMetricsEvaluator and ManifestEvaluator) #3118.

What changes are included in this PR?

InclusiveMetricsEvaluator::in and ManifestEvaluator::in tested the lower bound and the upper bound against the full literal set independently. An In list whose values sit entirely outside [lower, upper] but straddle it — for example bounds [30, 79] and id IN (5, 104) — was therefore not pruned.

Both evaluators now narrow the literal set against each bound in turn, matching Iceberg Java, PyIceberg, and the existing StrictMetricsEvaluator::not_in implementation in this crate. Scan results were already correct (the plan was a superset); this only avoids opening files and manifests that cannot contain a match.

Are these changes tested?

Unit tests in inclusive_metrics_evaluator and manifest_evaluator for the straddling case (id IN (5, 104) against bounds [30, 79]). Existing In tests in those modules still pass.

Locally: cargo fmt --all -- --check, cargo clippy -p iceberg --all-targets --all-features -- -D warnings, and cargo test -p iceberg --lib expr::visitors.

AI Disclosure

Assisted draft of the bound-narrowing change and regression tests. The algorithm matches Iceberg Java, PyIceberg, and StrictMetricsEvaluator::not_in. Reviewed and verified with the checks above.

[dhruvarya-db]

Looks like RowGroupMetricsEvaluator::in also has the same issue?

[M-Tesla]

Thanks — it did. RowGroupMetricsEvaluator::in now uses the same both-bounds check, with a regression test for float bounds [4.0, 6.0] and IN (2.0, 8.0).

[dhruvarya-db]

I wonder if something like might be more readable. We avoid creating a mutable filtered_literals as well this way

let lower_bound = self.lower_bound(field_id);
        let upper_bound = self.upper_bound(field_id);

        if lower_bound.is_some_and(Datum::is_nan) || upper_bound.is_some_and(Datum::is_nan) {
            return ROWS_MIGHT_MATCH;
        }

        let any_literal_in_bounds = match (lower_bound, upper_bound) {
            (Some(lower), Some(upper)) => {
                literals.iter().any(|datum| datum.ge(lower) && datum.le(upper))
            }
            (Some(lower), None) => literals.iter().any(|datum| datum.ge(lower)),
            (None, Some(upper)) => literals.iter().any(|datum| datum.le(upper)),
            (None, None) => true,
        };

        if !any_literal_in_bounds {
            return ROWS_CANNOT_MATCH;
        }

        ROWS_MIGHT_MATCH

[M-Tesla]

Agreed, that's cleaner. Switched to any() over the original set instead of clone + retain.

[dhruvarya-db]

Same applies here https://github.com/apache/iceberg-rust/pull/3144/changes#r3938316479

[M-Tesla]

Done here as well — ManifestEvaluator::in now uses the same any() form.

[M-Tesla]

Thanks @dhruvarya-db — applied your suggestions in b75d924: any() instead of clone/retain, and the same straddling-bounds prune in RowGroupMetricsEvaluator.

[dhruvarya-db]

LGTM (I am not a maintainer though)

[dhruvarya-db]

I wonder if it is worth it to factor out this pattern into a function and reuse it across the three callsites?

[M-Tesla]

Good call, the three in evaluators share that match. Pulled it into a small crate-private helper and left bound loading / NaN handling at each callsite.