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


##########
format/spec.md:
##########
@@ -1011,12 +1010,27 @@ For example, stats for an optional `geometry` field 
named `location` with field-
   10_804: optional long value_count;
   10_805: optional long null_value_count;
   10_807: optional long total_bytes;
-  // tight_bounds and nan_value_count are omitted for geo types
+  // nan_value_count is omitted for geo types
 }
 ```
 
 For `variant`, both bounds are unshredded `variant` that store variant field 
bounds by normalized JSON paths as field names. See [Bounds for 
Variant](#bounds-for-variant) for details on producing these bounds.
 
+###### Stats Tightness
+
+Statistics are _tight_ when the bounds, `null_value_count`, and 
`nan_value_count` are exact for the live rows of a file:
+
+* `lower_bound` and `upper_bound` for fixed-length types are equal to the 
minimum and maximum of the live values.
+* `null_value_count` and `nan_value_count` are the counts across the live rows.
+
+Bounds for `string` and `binary` are never tight because they may be truncated 
when written. Bounds for `geometry` and `geography` are a bounding box, not 
exact minimum and maximum values.
+
+`variant` bounds are stored per leaf field and are optional; a leaf field may 
have no bound, but any bound that is present is exact. A `variant` value is 
null only when the variant itself is null; a non-null variant that encodes a 
variant null is not counted as a null value.
+
+Statistics other than the bounds, `null_value_count`, and `nan_value_count` 
are not part of tightness. In particular, `value_count` is the number of values 
across all rows written to the file and is not adjusted when rows are deleted; 
for a field with no repeated (`list` or `map`) ancestor, the live value count 
is `value_count` minus the deletion vector's cardinality.

Review Comment:
   > for a field with no repeated (`list` or `map`) ancestor
   
   I don't know if "repeated ancestor" is an established terminology or not, 
and the property that matters is one value per row. Would the following read 
better?
   
   > for a field with one value per row (not nested `list` element or `map` 
key/value), the live value count is `value_count` minus the deletion vector's 
cardinality."
   
   I also want to point out that the current v4/Parquet implementation omits 
those metrics. 
   * v4 `StatsUtil` omits list-element and map key/value fields from 
`content_stats` 
([StatsUtil.java#L314-L327](https://github.com/apache/iceberg/blob/65cb42d6e6d850f8ee961cec70d0bf414f692376/core/src/main/java/org/apache/iceberg/StatsUtil.java#L314-L327)).
   * `ParquetMetrics` list and map visitors return an empty metrics list, so 
those fields get no metrics 
([ParquetMetrics.java#L220-L233](https://github.com/apache/iceberg/blob/65cb42d6e6d850f8ee961cec70d0bf414f692376/parquet/src/main/java/org/apache/iceberg/parquet/ParquetMetrics.java#L220-L233)).
   
   Should the spec say that stats are not stored for the list/map fields? or 
that should be left as an implementation choice?



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