Copilot commented on code in PR #686:
URL: https://github.com/apache/iceberg-cpp/pull/686#discussion_r3312234990
##########
src/iceberg/test/strict_metrics_evaluator_test.cc:
##########
@@ -846,4 +846,38 @@ TEST_F(StrictMetricsEvaluatorMigratedTest,
EvaluateOnNestedColumnWithStats) {
ExpectShouldRead(Expressions::NotNull("struct.nested_col_with_stats"),
false);
}
+TEST_F(StrictMetricsEvaluatorMigratedTest, MissingNullCountForField) {
+ // Field 14 (no_nan_stats, float64, optional) has bounds and value_counts
but is
+ // missing from null_value_counts. The evaluator must conservatively assume
nulls
+ // may exist and return kRowsMightNotMatch for comparison operators.
+ ExpectShouldRead(Expressions::LessThan("no_nan_stats",
Literal::Double(200.0)), false);
+ ExpectShouldRead(Expressions::LessThanOrEqual("no_nan_stats",
Literal::Double(200.0)),
+ false);
+ ExpectShouldRead(Expressions::GreaterThan("no_nan_stats",
Literal::Double(-1.0)),
+ false);
+ ExpectShouldRead(Expressions::GreaterThanOrEqual("no_nan_stats",
Literal::Double(-1.0)),
+ false);
+ ExpectShouldRead(Expressions::Equal("no_nan_stats", Literal::Double(50.0)),
false);
+}
Review Comment:
`MissingNullCountForField` doesn’t currently set lower/upper bounds for
field 14 in the `DataFile` being evaluated (it defaults to `file_`, where
bounds only exist for ids 1, 7, 12, 13, 17). Without bounds, these comparison
predicates will return `kRowsMightNotMatch` regardless of the null-count logic,
so the test won’t regress the original bug (it would pass even before the fix).
Consider constructing a `DataFile` that has bounds for field 14 while
`null_value_counts` is non-empty and *missing* entry 14 (and ensure NaN stats
for 14 are present/zero so the test isolates the null-count path).
--
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]
