stevenzwu commented on code in PR #17322:
URL: https://github.com/apache/iceberg/pull/17322#discussion_r3624857514


##########
core/src/test/java/org/apache/iceberg/TestContentStatsStruct.java:
##########
@@ -64,7 +71,7 @@ public class TestContentStatsStruct {
           true,
           26L,
           0L,
-          0L,
+          null,

Review Comment:
   Intentional: `data` is optional (tracks a null count, kept `0L` above) but 
not floating-point, so it has no NaN count. With 
`hasNullCount()`/`hasNaNCount()` now derived from a non-null boxed `Long`, an 
untracked count must be `null` — matching what `StatsUtil.fieldStatsStruct` 
declares. `ID_STATS` (a required long) is `null` for both.



##########
core/src/test/java/org/apache/iceberg/TestContentStatsStruct.java:
##########
@@ -103,7 +110,7 @@ public void testSetStatsUnknownField() {
     ContentStatsStruct stats = new ContentStatsStruct(CONTENT_STATS_STRUCT);
 
     FieldStats<Integer> fieldStats =
-        new FieldStatsStruct<>(UNKNOWN_FIELD_STATS_STRUCT, 0, 10, false, 8, 3, 
0, null);
+        new FieldStatsStruct<>(UNKNOWN_FIELD_STATS_STRUCT, 0, 10, false, 8, 
null, null, null);

Review Comment:
   Intentional: `UNKNOWN_FIELD_STATS_STRUCT` is a required Integer, so it 
declares neither count; both args are `null`. Previously `3, 0` — harmless 
under the old type-based `tracksStat`, but with the value-based check a 
non-null value would make `hasNullCount()`/`hasNaNCount()` report a metric the 
type doesn't track. This test never reads the counts, so `null` just keeps the 
fixture consistent.



-- 
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]

Reply via email to