anoopj commented on code in PR #18310:
URL: https://github.com/apache/iceberg/pull/18310#discussion_r4146645493
##########
core/src/main/java/org/apache/iceberg/GeometryFieldMetrics.java:
##########
@@ -69,15 +69,14 @@ public void addValue(ByteBuffer wkb) {
public GeometryFieldMetrics build() {
// build() returns null bounds when a dimension has no value or a value
could not be bounded,
- // which leaves the field with counts but no bounds. The average size is
still reported: it
- // does not depend on whether a box could be formed.
+ // which leaves the field with counts but no bounds. The total is still
reported and does not
Review Comment:
Minor: `total` is ambigous here.
##########
core/src/main/java/org/apache/iceberg/GeometryFieldMetrics.java:
##########
@@ -69,15 +69,14 @@ public void addValue(ByteBuffer wkb) {
public GeometryFieldMetrics build() {
// build() returns null bounds when a dimension has no value or a value
could not be bounded,
- // which leaves the field with counts but no bounds. The average size is
still reported: it
- // does not depend on whether a box could be formed.
+ // which leaves the field with counts but no bounds. The total is still
reported and does not
+ // depend on whether a box could be formed.
Review Comment:
```suggestion
// which leaves the field with counts but no bounds. The total bytes
are still reported
// and do not depend on whether a box could be formed.
```
##########
format/spec.md:
##########
@@ -827,7 +827,7 @@ Each stats struct holds statistics for one table field. It
may contain the follo
| _optional_ | 4 | `value_count` | `long`
| all | Number of values in the
column (including null and NaN values) |
| _optional_ | 5 | `null_value_count` | `long`
| optional fields | Number of null values in the
column |
| _optional_ | 6 | `nan_value_count` | `long`
| `float`, `double` | Number of NaN values in the
column |
-| _optional_ | 7 | `avg_value_size_in_bytes` | `int`
| `string`, `binary`, `variant`, `geometry`, `geography` | Avg value size in
memory (uncompressed) in bytes over non-null values to estimate memory
consumption |
Review Comment:
Is this stacked on top of #18308? We need to drop this otherwise, as it's
conflicting.
--
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]
