You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "anoopj (via GitHub)" <gi...@apache.org> on 2026/10/19 18:22:28 UTC
[PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
anoopj opened a new pull request, #18585:
URL: https://github.com/apache/iceberg/pull/18585
Track stats tightness with a single file-level `tight_stats` flag on the tracked file instead of a per-column `tight_bounds field`. The flag covers all statistics (bounds and `null`, `NaN`, and value counts) for the file's live rows.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "anoopj (via GitHub)" <gi...@apache.org>.
anoopj commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4319961545
##########
format/spec.md:
##########
@@ -1011,12 +1012,35 @@ 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.
+<a id="stats-tightness"></a>
+###### Stats Tightness
+
+Statistics are *tight* when they are exact for the live rows of a file:
+
+* Bounds are the minimum and maximum of the live values.
+* `null_value_count` and `nan_value_count` are the counts across the live rows.
+
+`value_count` and `total_bytes` are not part of tightness. `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. `total_bytes` is an estimate and is likewise not adjusted for deleted rows.
Review Comment:
So the derivation logic above to calculate `value_count` by subtracting dv cardinality is correct.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "anoopj (via GitHub)" <gi...@apache.org>.
anoopj commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4318573591
##########
format/spec.md:
##########
@@ -1011,12 +1012,26 @@ 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.
+<a id="stats-tightness"></a>
+###### Stats Tightness
+
+Statistics are *tight* when they are exact for the live rows of a file: bounds are the minimum and maximum of the live values and the counts (`value_count`, `null_value_count`, and `nan_value_count`) are exact. When `tight_stats` is absent, readers must assume stats are not tight.
+
+When `tight_stats` is true:
+
+* Counts are exact for all fields.
+* Bounds are exact for fixed-width primitives and for `variant`.
Review Comment:
Good callout. I think variant is a bit different in the sense that some of the fields may not have stats, but any field level stats present has the same tightness guarantees. I will try to incorporate this.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "anoopj (via GitHub)" <gi...@apache.org>.
anoopj commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4338783539
##########
format/spec.md:
##########
@@ -949,7 +950,6 @@ Each stats struct holds statistics for one table field. It may contain the follo
|-------------|--------|---------------------------|---------------------------|-----------------------------------------------|-------------|
| _optional_ | 1 | `lower_bound` | Field type or `geo_lower` | all primitives or `variant` | Lower bound stored as the field's type, or `geo_lower` for geo types |
| _optional_ | 2 | `upper_bound` | Field type or `geo_upper` | all primitives or `variant` | Upper bound stored as the field's type, or `geo_upper` for geo types |
-| _optional_ | 3 | `tight_bounds` | `boolean` | all primitives except for `geometry` and `geography` | When true, `lower_bound` and `upper_bound` must be equal to the min and max values |
Review Comment:
Thanks for catching that. Done.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4328557164
##########
format/spec.md:
##########
@@ -1011,12 +1012,24 @@ 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:
+
+* Bounds are the minimum and maximum of the live values. Bounds for `string` and `binary` are never tight because they may be truncated when written, and bounds for `geometry` and `geography` are a bounding box, not exact minimum and maximum values.
Review Comment:
> * Bounds for fixed-width types are the minimum and maximum for live values
The second sentence should not be in the definition, it is a clarification that can come later. The idea is to be clear and direct and "fixed-width types" says what we need to. Leaving aside the specific cases makes this easier to understand.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "anoopj (via GitHub)" <gi...@apache.org>.
anoopj commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4328999332
##########
format/spec.md:
##########
@@ -1011,12 +1012,24 @@ 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:
Review Comment:
Italic - was used to using single asterisk for italics. Changed to underscores to be 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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4320114841
##########
format/spec.md:
##########
@@ -1011,12 +1012,35 @@ 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.
+<a id="stats-tightness"></a>
+###### Stats Tightness
+
+Statistics are *tight* when they are exact for the live rows of a file:
+
+* Bounds are the minimum and maximum of the live values.
+* `null_value_count` and `nan_value_count` are the counts across the live rows.
+
+`value_count` and `total_bytes` are not part of tightness. `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. `total_bytes` is an estimate and is likewise not adjusted for deleted rows.
+
+When `tight_stats` is absent, readers must assume stats are not tight.
+
+When `tight_stats` is true:
+
+* `null_value_count` and `nan_value_count` are exact for the live rows.
+* Bounds are exact for primitive types other than `string` and `binary`, whose bounds may have been truncated when written.
Review Comment:
I think this should also be part of the definition above. Bounds for string and binary fields are excluded and are never tight.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4328577156
##########
format/spec.md:
##########
@@ -1011,12 +1012,24 @@ 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:
+
+* Bounds are the minimum and maximum of the live values. Bounds for `string` and `binary` are never tight because they may be truncated when written, and 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.
+* `null_value_count` and `nan_value_count` are the counts across the live rows. 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.
Review Comment:
This should be the second bullet point.
Information about variant can be its own paragraph below. We don't need to bring exceptions in when defining what it means for bounds to be exact.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4328504547
##########
format/spec.md:
##########
@@ -868,6 +869,8 @@ The contents of a file are part of a snapshot if its tracking `status` is **live
- `column_files`:
- `column_files` may only be set if the tracked file is a data file or a v4 leaf manifest.
- A field ID may appear in `field_ids` of at most one column file in a tracked file's `column_files`.
+ - `tight_stats`:
+ - `tight_stats` may be true only if `content_stats` are tight. See [Stats Tightness](#stats-tightness).
Review Comment:
This restates the description. I would remove it.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "anoopj (via GitHub)" <gi...@apache.org>.
anoopj commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4329003082
##########
format/spec.md:
##########
@@ -1011,12 +1012,24 @@ 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:
+
+* Bounds are the minimum and maximum of the live values. Bounds for `string` and `binary` are never tight because they may be truncated when written, and bounds for `geometry` and `geography` are a bounding box, not exact minimum and maximum values.
Review Comment:
That is a lot better. Thank you!
##########
format/spec.md:
##########
@@ -1011,12 +1012,24 @@ 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:
+
+* Bounds are the minimum and maximum of the live values. Bounds for `string` and `binary` are never tight because they may be truncated when written, and 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.
Review Comment:
Good idea. Done.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "anoopj (via GitHub)" <gi...@apache.org>.
anoopj commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4328991662
##########
format/spec.md:
##########
@@ -868,6 +869,8 @@ The contents of a file are part of a snapshot if its tracking `status` is **live
- `column_files`:
- `column_files` may only be set if the tracked file is a data file or a v4 leaf manifest.
- A field ID may appear in `field_ids` of at most one column file in a tracked file's `column_files`.
+ - `tight_stats`:
+ - `tight_stats` may be true only if `content_stats` are tight. See [Stats Tightness](#stats-tightness).
Review Comment:
Removed it.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "anoopj (via GitHub)" <gi...@apache.org>.
anoopj commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4338860454
##########
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:
Thanks for the feedback, @stevenzwu ! Repeated is a Dremel terminology I'm a bit used to, but you are right that it's not in the spec. I reworded as you suggested.
> Should the spec say that stats are not stored for the list/map fields? or that should be left as an implementation choice?
I think we should leave it as an implementation choice, as we do today.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "amogh-jahagirdar (via GitHub)" <gi...@apache.org>.
amogh-jahagirdar commented on PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#issuecomment-6452998710
Thanks @anoopj and all for reviewing!
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4333556347
##########
format/spec.md:
##########
@@ -949,7 +950,6 @@ Each stats struct holds statistics for one table field. It may contain the follo
|-------------|--------|---------------------------|---------------------------|-----------------------------------------------|-------------|
| _optional_ | 1 | `lower_bound` | Field type or `geo_lower` | all primitives or `variant` | Lower bound stored as the field's type, or `geo_lower` for geo types |
| _optional_ | 2 | `upper_bound` | Field type or `geo_upper` | all primitives or `variant` | Upper bound stored as the field's type, or `geo_upper` for geo types |
-| _optional_ | 3 | `tight_bounds` | `boolean` | all primitives except for `geometry` and `geography` | When true, `lower_bound` and `upper_bound` must be equal to the min and max values |
Review Comment:
are we going to adjust the offsets?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4339249969
##########
format/spec.md:
##########
@@ -949,7 +950,6 @@ Each stats struct holds statistics for one table field. It may contain the follo
|-------------|--------|---------------------------|---------------------------|-----------------------------------------------|-------------|
| _optional_ | 1 | `lower_bound` | Field type or `geo_lower` | all primitives or `variant` | Lower bound stored as the field's type, or `geo_lower` for geo types |
| _optional_ | 2 | `upper_bound` | Field type or `geo_upper` | all primitives or `variant` | Upper bound stored as the field's type, or `geo_upper` for geo types |
-| _optional_ | 3 | `tight_bounds` | `boolean` | all primitives except for `geometry` and `geography` | When true, `lower_bound` and `upper_bound` must be equal to the min and max values |
Review Comment:
my thinking is that the Spec is still in development and not finalized, so it seems weird to have offset 3 not being used for anything just because we decided to move things around. I'd rather adjust the offsets now instead of later before voting on the v4 spec and realize that one of the offsets isn't used, wdyt?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4342293009
##########
format/spec.md:
##########
@@ -1011,12 +1012,35 @@ 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.
+<a id="stats-tightness"></a>
+###### Stats Tightness
+
+Statistics are *tight* when they are exact for the live rows of a file:
+
+* Bounds are the minimum and maximum of the live values.
+* `null_value_count` and `nan_value_count` are the counts across the live rows.
+
+`value_count` and `total_bytes` are not part of tightness. `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. `total_bytes` is an estimate and is likewise not adjusted for deleted rows.
Review Comment:
Following up here, we realized that this has a negative impact on filtering. Updating `null_value_count` but not `value_count` makes the filtering logic dependent on both `dv.cardinality()` and the `tight_stats` field being true. We've decided that it isn't worth the optimization of not updating `value_count` so we are adding it back to `tight_stats`.
There's a more detailed write-up here for anyone interested: https://gist.github.com/rdblue/65314d6378e3de517a421d57c9da00e1
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "anoopj (via GitHub)" <gi...@apache.org>.
anoopj commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4339970694
##########
format/spec.md:
##########
@@ -949,7 +950,6 @@ Each stats struct holds statistics for one table field. It may contain the follo
|-------------|--------|---------------------------|---------------------------|-----------------------------------------------|-------------|
| _optional_ | 1 | `lower_bound` | Field type or `geo_lower` | all primitives or `variant` | Lower bound stored as the field's type, or `geo_lower` for geo types |
| _optional_ | 2 | `upper_bound` | Field type or `geo_upper` | all primitives or `variant` | Upper bound stored as the field's type, or `geo_upper` for geo types |
-| _optional_ | 3 | `tight_bounds` | `boolean` | all primitives except for `geometry` and `geography` | When true, `lower_bound` and `upper_bound` must be equal to the min and max values |
Review Comment:
I was thinking along the lines of what @nastra said, but can revert the adjustment if you feel strongly, @rdblue
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "anoopj (via GitHub)" <gi...@apache.org>.
anoopj commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4329028380
##########
format/spec.md:
##########
@@ -1011,12 +1012,24 @@ 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:
+
+* Bounds are the minimum and maximum of the live values. Bounds for `string` and `binary` are never tight because they may be truncated when written, and 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.
+* `null_value_count` and `nan_value_count` are the counts across the live rows. 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.
Review Comment:
Done. Variant is it's own paragraph.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "anoopj (via GitHub)" <gi...@apache.org>.
anoopj commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4328132869
##########
format/spec.md:
##########
@@ -1011,12 +1012,35 @@ 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.
+<a id="stats-tightness"></a>
+###### Stats Tightness
+
+Statistics are *tight* when they are exact for the live rows of a file:
Review Comment:
Done. The definition now names them (bounds, null and NaN counts). The rest are clearly out.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "anoopj (via GitHub)" <gi...@apache.org>.
anoopj commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4328119296
##########
format/spec.md:
##########
@@ -868,6 +869,8 @@ The contents of a file are part of a snapshot if its tracking `status` is **live
- `column_files`:
- `column_files` may only be set if the tracked file is a data file or a v4 leaf manifest.
- A field ID may appear in `field_ids` of at most one column file in a tracked file's `column_files`.
+ - `tight_stats`:
+ - `tight_stats` may be true only if `content_stats` are tight; attaching or updating a `deletion_vector` without recomputing stats makes them no longer tight. See [Stats Tightness](#stats-tightness).
Review Comment:
Removed.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "dramaticlly (via GitHub)" <gi...@apache.org>.
dramaticlly commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4329477154
##########
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-width 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.
+
+When `tight_stats` is absent, readers must assume stats are not tight. A deletion vector does not by itself make a file's stats non-tight; `tight_stats` is the only indicator of tightness. Stats may be recomputed to restore tightness without rewriting the data file.
Review Comment:
I am not sure if we need to mention the DV case here
> A deletion vector does not by itself make a file's stats non-tight.
when `tight_stats` is set, the value present tightness of the statistics. missing value must be treated as not tight by all readers. (must or should, still debating the word on this as spec cannot really enforce the reader behavior)
##########
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-width types are equal to the minimum and maximum of the live values.
Review Comment:
existing spec language seem to refer "variable length types", I think we shall pick either width or length to be 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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "anoopj (via GitHub)" <gi...@apache.org>.
anoopj commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4328163929
##########
format/spec.md:
##########
@@ -1011,12 +1012,35 @@ 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.
+<a id="stats-tightness"></a>
+###### Stats Tightness
+
+Statistics are *tight* when they are exact for the live rows of a file:
+
+* Bounds are the minimum and maximum of the live values.
+* `null_value_count` and `nan_value_count` are the counts across the live rows.
+
+`value_count` and `total_bytes` are not part of tightness. `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. `total_bytes` is an estimate and is likewise not adjusted for deleted rows.
+
+When `tight_stats` is absent, readers must assume stats are not tight.
+
+When `tight_stats` is true:
+
+* `null_value_count` and `nan_value_count` are exact for the live rows.
+* Bounds are exact for primitive types other than `string` and `binary`, whose bounds may have been truncated when written.
+* `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.
+* `geometry` and `geography` bounds are a bounding box that contains the live values, not exact minimum and maximum values.
+
+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. Variants do not have a `nan_value_count`.
Review Comment:
Thanks. I dropped the `NaN`-count line, kept the variant-null clause.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4328557164
##########
format/spec.md:
##########
@@ -1011,12 +1012,24 @@ 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:
+
+* Bounds are the minimum and maximum of the live values. Bounds for `string` and `binary` are never tight because they may be truncated when written, and bounds for `geometry` and `geography` are a bounding box, not exact minimum and maximum values.
Review Comment:
> * `lower_bound` and `upper_bound` for fixed-width types are equal to the minimum and maximum live values
The second sentence should not be in the definition, it is a clarification that can come later. The idea is to be clear and direct and "fixed-width types" says what we need to. Leaving aside the specific cases makes this easier to understand.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4322075578
##########
format/spec.md:
##########
@@ -1011,12 +1012,35 @@ 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.
+<a id="stats-tightness"></a>
Review Comment:
yeah we shouldn't need to generate this
##########
format/spec.md:
##########
@@ -1011,12 +1012,35 @@ 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.
+<a id="stats-tightness"></a>
+###### Stats Tightness
+
+Statistics are *tight* when they are exact for the live rows of a file:
+
+* Bounds are the minimum and maximum of the live values.
+* `null_value_count` and `nan_value_count` are the counts across the live rows.
+
+`value_count` and `total_bytes` are not part of tightness. `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. `total_bytes` is an estimate and is likewise not adjusted for deleted rows.
+
+When `tight_stats` is absent, readers must assume stats are not tight.
+
+When `tight_stats` is true:
+
+* `null_value_count` and `nan_value_count` are exact for the live rows.
+* Bounds are exact for primitive types other than `string` and `binary`, whose bounds may have been truncated when written.
Review Comment:
+1
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4342298106
##########
format/spec.md:
##########
@@ -1011,12 +1012,35 @@ 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.
+<a id="stats-tightness"></a>
+###### Stats Tightness
+
+Statistics are *tight* when they are exact for the live rows of a file:
+
+* Bounds are the minimum and maximum of the live values.
+* `null_value_count` and `nan_value_count` are the counts across the live rows.
+
+`value_count` and `total_bytes` are not part of tightness. `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. `total_bytes` is an estimate and is likewise not adjusted for deleted rows.
Review Comment:
I'm going to resolve this thread since we've decided to keep `value_count` updated for stats to be tight. See this comment: https://github.com/apache/iceberg/pull/18585#discussion_r4342293009
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "anoopj (via GitHub)" <gi...@apache.org>.
anoopj commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4328160570
##########
format/spec.md:
##########
@@ -1011,12 +1012,35 @@ 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.
+<a id="stats-tightness"></a>
+###### Stats Tightness
+
+Statistics are *tight* when they are exact for the live rows of a file:
+
+* Bounds are the minimum and maximum of the live values.
+* `null_value_count` and `nan_value_count` are the counts across the live rows.
+
+`value_count` and `total_bytes` are not part of tightness. `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. `total_bytes` is an estimate and is likewise not adjusted for deleted rows.
+
+When `tight_stats` is absent, readers must assume stats are not tight.
+
+When `tight_stats` is true:
+
+* `null_value_count` and `nan_value_count` are exact for the live rows.
+* Bounds are exact for primitive types other than `string` and `binary`, whose bounds may have been truncated when written.
Review Comment:
Moved into the definition. I folded geo in there too, since it's the same kind of exception. Keeping it under the `tight_stats` was misleading.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "anoopj (via GitHub)" <gi...@apache.org>.
anoopj commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4328123804
##########
format/spec.md:
##########
@@ -1011,12 +1012,35 @@ 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.
+<a id="stats-tightness"></a>
Review Comment:
Looks like the anchor tag will be autogenerated for headings. I have removed it.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "amogh-jahagirdar (via GitHub)" <gi...@apache.org>.
amogh-jahagirdar merged PR #18585:
URL: https://github.com/apache/iceberg/pull/18585
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "anoopj (via GitHub)" <gi...@apache.org>.
anoopj commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4319968955
##########
format/spec.md:
##########
@@ -1011,12 +1012,35 @@ 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.
+<a id="stats-tightness"></a>
+###### Stats Tightness
+
+Statistics are *tight* when they are exact for the live rows of a file:
+
+* Bounds are the minimum and maximum of the live values.
+* `null_value_count` and `nan_value_count` are the counts across the live rows.
+
+`value_count` and `total_bytes` are not part of tightness. `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. `total_bytes` is an estimate and is likewise not adjusted for deleted rows.
Review Comment:
Not spelled out here, but similarly non-null values can be calculated by value_count - dv_cardinality - null_count (assuming no repeated ancestors).
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4320091025
##########
format/spec.md:
##########
@@ -868,6 +869,8 @@ The contents of a file are part of a snapshot if its tracking `status` is **live
- `column_files`:
- `column_files` may only be set if the tracked file is a data file or a v4 leaf manifest.
- A field ID may appear in `field_ids` of at most one column file in a tracked file's `column_files`.
+ - `tight_stats`:
+ - `tight_stats` may be true only if `content_stats` are tight; attaching or updating a `deletion_vector` without recomputing stats makes them no longer tight. See [Stats Tightness](#stats-tightness).
Review Comment:
Do we need to mention a DV here? You don't necessarily need to recompute stats and getting into the nuance should be done in more detail below. I'd probably just remove that sentence.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4320109808
##########
format/spec.md:
##########
@@ -1011,12 +1012,35 @@ 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.
+<a id="stats-tightness"></a>
+###### Stats Tightness
+
+Statistics are *tight* when they are exact for the live rows of a file:
+
+* Bounds are the minimum and maximum of the live values.
+* `null_value_count` and `nan_value_count` are the counts across the live rows.
+
+`value_count` and `total_bytes` are not part of tightness. `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. `total_bytes` is an estimate and is likewise not adjusted for deleted rows.
+
+When `tight_stats` is absent, readers must assume stats are not tight.
+
+When `tight_stats` is true:
+
+* `null_value_count` and `nan_value_count` are exact for the live rows.
Review Comment:
This is restating the definition above and should be removed.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "anoopj (via GitHub)" <gi...@apache.org>.
anoopj commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4351085447
##########
format/spec.md:
##########
@@ -1008,15 +1007,32 @@ For example, stats for an optional `geometry` field named `location` with field-
10_816: optional double z;
10_817: optional double m;
}
- 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
+ 10_803: optional long value_count;
+ 10_804: optional long null_value_count;
+ 10_806: optional long total_bytes;
+ // 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.
+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.
+
+###### Stats Tightness
+
+Statistics are _tight_ when the bounds, `value_count`, `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.
+* `value_count`, `null_value_count`, and `nan_value_count` are the counts across the live rows.
+
+Regardless of `tight_stats`, bounds for `string` and `binary` are never tight because they may be truncated when written, and bounds for `geometry` and `geography` are never tight because they are a bounding box rather than exact minimum and maximum values.
+
+For `variant`, `lower_bound` and `upper_bound` hold one bound per normalized JSON path. A path may be omitted, which means that path has no bound. Whether a present bound is tight follows the same rules as above for that path's value type.
+
+Statistics other than the bounds, `value_count`, `null_value_count`, and `nan_value_count` are not part of tightness.
+
+When `tight_stats` is absent, readers must assume stats are not tight; `tight_stats` is the only indicator of tightness. Stats may be recomputed to restore tightness without rewriting the data file.
Review Comment:
Remnant from the time when the spec was talking about DVs. Removed
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "anoopj (via GitHub)" <gi...@apache.org>.
anoopj commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4343081763
##########
format/spec.md:
##########
@@ -1008,15 +1007,30 @@ For example, stats for an optional `geometry` field named `location` with field-
10_816: optional double z;
10_817: optional double m;
}
- 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
+ 10_803: optional long value_count;
+ 10_804: optional long null_value_count;
+ 10_806: optional long total_bytes;
+ // 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, and each present bound follows the rules above for its type. 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.
Review Comment:
Thanks for the suggestions.
Moved the variant null out of the tightness section up next to the variant content-stats description, since it defines null_value_count semantics rather than tightness.
About leaf fields, I used your wording.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "anoopj (via GitHub)" <gi...@apache.org>.
anoopj commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4329837740
##########
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.
+
+When `tight_stats` is absent, readers must assume stats are not tight; `tight_stats` is the only indicator of tightness. Stats may be recomputed to restore tightness without rewriting the data file.
Review Comment:
Omitting this for now, since that is probably too detailed
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "anoopj (via GitHub)" <gi...@apache.org>.
anoopj commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4329832126
##########
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.
+
+When `tight_stats` is absent, readers must assume stats are not tight; `tight_stats` is the only indicator of tightness. Stats may be recomputed to restore tightness without rewriting the data file.
Review Comment:
Readers without context on DVs may be wondering why anyone may need to rewrite data files. If needed we can bring DV as an example. e.g. something like
> Stats that are not tight (e.g. after a deletion vector is written) may be recomputed to restore tightness without rewriting the data file.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "anoopj (via GitHub)" <gi...@apache.org>.
anoopj commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4329778790
##########
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-width 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.
+
+When `tight_stats` is absent, readers must assume stats are not tight. A deletion vector does not by itself make a file's stats non-tight; `tight_stats` is the only indicator of tightness. Stats may be recomputed to restore tightness without rewriting the data file.
Review Comment:
Agreed, it's redundant with "tight_stats is the only indicator of tightness", so I dropped the DV sentence.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "anoopj (via GitHub)" <gi...@apache.org>.
anoopj commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4328143903
##########
format/spec.md:
##########
@@ -1011,12 +1012,35 @@ 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.
+<a id="stats-tightness"></a>
+###### Stats Tightness
+
+Statistics are *tight* when they are exact for the live rows of a file:
+
+* Bounds are the minimum and maximum of the live values.
+* `null_value_count` and `nan_value_count` are the counts across the live rows.
+
+`value_count` and `total_bytes` are not part of tightness. `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. `total_bytes` is an estimate and is likewise not adjusted for deleted rows.
Review Comment:
Done. Dropped the `total_bytes` sentence and reframed it as:
> Statistics other than the bounds, null_value_count, and nan_value_count are not part of tightness.
##########
format/spec.md:
##########
@@ -1011,12 +1012,35 @@ 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.
+<a id="stats-tightness"></a>
+###### Stats Tightness
+
+Statistics are *tight* when they are exact for the live rows of a file:
+
+* Bounds are the minimum and maximum of the live values.
+* `null_value_count` and `nan_value_count` are the counts across the live rows.
+
+`value_count` and `total_bytes` are not part of tightness. `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. `total_bytes` is an estimate and is likewise not adjusted for deleted rows.
+
+When `tight_stats` is absent, readers must assume stats are not tight.
+
+When `tight_stats` is true:
+
+* `null_value_count` and `nan_value_count` are exact for the live rows.
Review Comment:
Removed.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4342312998
##########
format/spec.md:
##########
@@ -949,7 +950,6 @@ Each stats struct holds statistics for one table field. It may contain the follo
|-------------|--------|---------------------------|---------------------------|-----------------------------------------------|-------------|
| _optional_ | 1 | `lower_bound` | Field type or `geo_lower` | all primitives or `variant` | Lower bound stored as the field's type, or `geo_lower` for geo types |
| _optional_ | 2 | `upper_bound` | Field type or `geo_upper` | all primitives or `variant` | Upper bound stored as the field's type, or `geo_upper` for geo types |
-| _optional_ | 3 | `tight_bounds` | `boolean` | all primitives except for `geometry` and `geography` | When true, `lower_bound` and `upper_bound` must be equal to the min and max values |
Review Comment:
We can always use 3 later. I'd leave it as-is but it's fine either way.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4328609638
##########
format/spec.md:
##########
@@ -1011,12 +1012,24 @@ 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:
+
+* Bounds are the minimum and maximum of the live values. Bounds for `string` and `binary` are never tight because they may be truncated when written, and 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.
Review Comment:
Also move this to a separate paragraph about variants.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "anoopj (via GitHub)" <gi...@apache.org>.
anoopj commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4329771535
##########
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-width types are equal to the minimum and maximum of the live values.
Review Comment:
Agree. Done.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
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)).
Do we need to clarify anything in the spec for the list/map fields?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4340444450
##########
format/spec.md:
##########
@@ -1008,15 +1007,30 @@ For example, stats for an optional `geometry` field named `location` with field-
10_816: optional double z;
10_817: optional double m;
}
- 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
+ 10_803: optional long value_count;
+ 10_804: optional long null_value_count;
+ 10_806: optional long total_bytes;
+ // 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, and each present bound follows the rules above for its type. 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.
Review Comment:
> 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.
This sentence defines what `null_value_count` includes for variant type, so it doesn't belong in the tightness section. If we want to clarify it, it fits with the `null_value_count` definition in [Field-level Metrics and Statistics](https://iceberg.apache.org/spec/#field-level-metrics-and-statistics), next to [Bounds for Variant](https://iceberg.apache.org/spec/#bounds-for-variant).
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4320095873
##########
format/spec.md:
##########
@@ -1011,12 +1012,35 @@ 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.
+<a id="stats-tightness"></a>
+###### Stats Tightness
+
+Statistics are *tight* when they are exact for the live rows of a file:
Review Comment:
When bounds, null count, and nan count are ...
Otherwise this is confusing for the other stats fields.
##########
format/spec.md:
##########
@@ -1011,12 +1012,35 @@ 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.
+<a id="stats-tightness"></a>
+###### Stats Tightness
+
+Statistics are *tight* when they are exact for the live rows of a file:
+
+* Bounds are the minimum and maximum of the live values.
+* `null_value_count` and `nan_value_count` are the counts across the live rows.
+
+`value_count` and `total_bytes` are not part of tightness. `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. `total_bytes` is an estimate and is likewise not adjusted for deleted rows.
Review Comment:
"Note that `value_count` is not included in tightness"
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4320118565
##########
format/spec.md:
##########
@@ -1011,12 +1012,35 @@ 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.
+<a id="stats-tightness"></a>
+###### Stats Tightness
+
+Statistics are *tight* when they are exact for the live rows of a file:
+
+* Bounds are the minimum and maximum of the live values.
+* `null_value_count` and `nan_value_count` are the counts across the live rows.
+
+`value_count` and `total_bytes` are not part of tightness. `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. `total_bytes` is an estimate and is likewise not adjusted for deleted rows.
+
+When `tight_stats` is absent, readers must assume stats are not tight.
+
+When `tight_stats` is true:
+
+* `null_value_count` and `nan_value_count` are exact for the live rows.
+* Bounds are exact for primitive types other than `string` and `binary`, whose bounds may have been truncated when written.
+* `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.
+* `geometry` and `geography` bounds are a bounding box that contains the live values, not exact minimum and maximum values.
+
+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. Variants do not have a `nan_value_count`.
Review Comment:
This is good. We don't need to specify that variants don't have a NaN count though.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4339057924
##########
format/spec.md:
##########
@@ -949,7 +950,6 @@ Each stats struct holds statistics for one table field. It may contain the follo
|-------------|--------|---------------------------|---------------------------|-----------------------------------------------|-------------|
| _optional_ | 1 | `lower_bound` | Field type or `geo_lower` | all primitives or `variant` | Lower bound stored as the field's type, or `geo_lower` for geo types |
| _optional_ | 2 | `upper_bound` | Field type or `geo_upper` | all primitives or `variant` | Upper bound stored as the field's type, or `geo_upper` for geo types |
-| _optional_ | 3 | `tight_bounds` | `boolean` | all primitives except for `geometry` and `geography` | When true, `lower_bound` and `upper_bound` must be equal to the min and max values |
Review Comment:
I thought that we weren't going to adjust the offsets. What do we gain by adjusting them? There is already at least one implementation so I have a slight preference to not adjusting them.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4340444450
##########
format/spec.md:
##########
@@ -1008,15 +1007,30 @@ For example, stats for an optional `geometry` field named `location` with field-
10_816: optional double z;
10_817: optional double m;
}
- 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
+ 10_803: optional long value_count;
+ 10_804: optional long null_value_count;
+ 10_806: optional long total_bytes;
+ // 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, and each present bound follows the rules above for its type. 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.
Review Comment:
> 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.
This sentence defines what `null_value_count` includes, so it doesn't belong in the tightness section. If we want to clarify it, it fits with the `null_value_count` definition in [Field-level Metrics and Statistics](https://iceberg.apache.org/spec/#field-level-metrics-and-statistics), next to [Bounds for Variant](https://iceberg.apache.org/spec/#bounds-for-variant).
##########
format/spec.md:
##########
@@ -1008,15 +1007,30 @@ For example, stats for an optional `geometry` field named `location` with field-
10_816: optional double z;
10_817: optional double m;
}
- 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
+ 10_803: optional long value_count;
+ 10_804: optional long null_value_count;
+ 10_806: optional long total_bytes;
+ // 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.
Review Comment:
> Regardless of `tight_stats` value, bounds for `string` and `binary` are never tight because they may be truncated when written.
It might be a bit more clear by adding the qualifying phrase: "Regardless of `tight_stats` value, ".
> Bounds for `geometry` and `geography` are a bounding box, not exact minimum and maximum values.
The bounds section already defines these as bounding boxes. What does `tight_stats` require for `geometry` and `geography` bounds? Are they tight when the bounding box is computed exactly from the live rows, or are they never tight like `string` and `binary` bounds? Could we state that explicitly?
##########
format/spec.md:
##########
@@ -1008,15 +1007,30 @@ For example, stats for an optional `geometry` field named `location` with field-
10_816: optional double z;
10_817: optional double m;
}
- 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
+ 10_803: optional long value_count;
+ 10_804: optional long null_value_count;
+ 10_806: optional long total_bytes;
+ // 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, and each present bound follows the rules above for its type. 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.
Review Comment:
> `variant` bounds are stored per leaf field and are optional; a leaf field may have no bound, and each present bound follows the rules above for its type.
I found it hard to understand, mainly because `leaf field` is not a defined term in the spec. Would the rephrasing below read a bit more clearly?
> For `variant`, `lower_bound` and `upper_bound` hold one bound per normalized JSON path. A path may be omitted, which means that path has no bound. Whether a present bound is tight follows the same rules as above for that path's value type.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4340444450
##########
format/spec.md:
##########
@@ -1008,15 +1007,30 @@ For example, stats for an optional `geometry` field named `location` with field-
10_816: optional double z;
10_817: optional double m;
}
- 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
+ 10_803: optional long value_count;
+ 10_804: optional long null_value_count;
+ 10_806: optional long total_bytes;
+ // 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, and each present bound follows the rules above for its type. 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.
Review Comment:
> 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.
This sentence defines what `null_value_count` includes for variant type and it doesn't touch tightness at all, so it doesn't belong in the tightness section. If we want to clarify it, it fits with the `null_value_count` definition in [Field-level Metrics and Statistics](https://iceberg.apache.org/spec/#field-level-metrics-and-statistics), next to [Bounds for Variant](https://iceberg.apache.org/spec/#bounds-for-variant).
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "anoopj (via GitHub)" <gi...@apache.org>.
anoopj commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4343060091
##########
format/spec.md:
##########
@@ -1008,15 +1007,30 @@ For example, stats for an optional `geometry` field named `location` with field-
10_816: optional double z;
10_817: optional double m;
}
- 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
+ 10_803: optional long value_count;
+ 10_804: optional long null_value_count;
+ 10_806: optional long total_bytes;
+ // 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.
Review Comment:
Added the qualifying phrase.
Geo - they are never tight like string/binary bounds. Clarified that as well.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4343729259
##########
format/spec.md:
##########
@@ -1008,15 +1007,32 @@ For example, stats for an optional `geometry` field named `location` with field-
10_816: optional double z;
10_817: optional double m;
}
- 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
+ 10_803: optional long value_count;
+ 10_804: optional long null_value_count;
+ 10_806: optional long total_bytes;
+ // 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.
+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.
+
+###### Stats Tightness
+
+Statistics are _tight_ when the bounds, `value_count`, `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.
+* `value_count`, `null_value_count`, and `nan_value_count` are the counts across the live rows.
+
+Regardless of `tight_stats`, bounds for `string` and `binary` are never tight because they may be truncated when written, and bounds for `geometry` and `geography` are never tight because they are a bounding box rather than exact minimum and maximum values.
+
+For `variant`, `lower_bound` and `upper_bound` hold one bound per normalized JSON path. A path may be omitted, which means that path has no bound. Whether a present bound is tight follows the same rules as above for that path's value type.
+
+Statistics other than the bounds, `value_count`, `null_value_count`, and `nan_value_count` are not part of tightness.
+
+When `tight_stats` is absent, readers must assume stats are not tight; `tight_stats` is the only indicator of tightness. Stats may be recomputed to restore tightness without rewriting the data file.
Review Comment:
> ; `tight_stats` is the only indicator of tightness
this sentence is probably not needed
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4328529679
##########
format/spec.md:
##########
@@ -1011,12 +1012,24 @@ 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:
Review Comment:
Is the intent to use bold or italic? Most other places use `_underscores_` for italic or `**double asterisks**` for bold.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "dramaticlly (via GitHub)" <gi...@apache.org>.
dramaticlly commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4317645305
##########
format/spec.md:
##########
@@ -1011,12 +1012,26 @@ 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.
+<a id="stats-tightness"></a>
+###### Stats Tightness
+
+Statistics are *tight* when they are exact for the live rows of a file: bounds are the minimum and maximum of the live values and the counts (`value_count`, `null_value_count`, and `nan_value_count`) are exact. When `tight_stats` is absent, readers must assume stats are not tight.
+
+When `tight_stats` is true:
+
+* Counts are exact for all fields.
+* Bounds are exact for fixed-width primitives and for `variant`.
Review Comment:
do we consider the variant bounds to be exact?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "anoopj (via GitHub)" <gi...@apache.org>.
anoopj commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4319885739
##########
format/spec.md:
##########
@@ -1011,12 +1012,35 @@ 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.
+<a id="stats-tightness"></a>
+###### Stats Tightness
+
+Statistics are *tight* when they are exact for the live rows of a file:
+
+* Bounds are the minimum and maximum of the live values.
+* `null_value_count` and `nan_value_count` are the counts across the live rows.
+
+`value_count` and `total_bytes` are not part of tightness. `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. `total_bytes` is an estimate and is likewise not adjusted for deleted rows.
Review Comment:
Verified against the [implementation](https://github.com/apache/iceberg/blob/ddbd99073c9d2d1d8d4fe32e1cfccee7b2dd2151/parquet/src/main/java/org/apache/iceberg/parquet/ParquetMetrics.java#L317-L334) that computes the `value_count`:
- `value_count` counts every row, including rows where a parent struct is null
- Parquet's per-column value count is one entry per row regardless of definition level, so a row whose parent struct is null still contributes to the leaf's value_count.
- Parquet marks a leaf null whenever its definition level is below max, which includes the case where an ancestor struct is null, so a null parent also increments the leaf's null_value_count.
Repeated (list/map) fields collect no stats at all
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4320106874
##########
format/spec.md:
##########
@@ -1011,12 +1012,35 @@ 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.
+<a id="stats-tightness"></a>
+###### Stats Tightness
+
+Statistics are *tight* when they are exact for the live rows of a file:
+
+* Bounds are the minimum and maximum of the live values.
+* `null_value_count` and `nan_value_count` are the counts across the live rows.
+
+`value_count` and `total_bytes` are not part of tightness. `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. `total_bytes` is an estimate and is likewise not adjusted for deleted rows.
Review Comment:
We don't need to state that total bytes is excluded. We should take care of that with the definition of tightness that only mentions null and nan counts in addition to the bounds. All other fields are not tight and we can't require new ones to be included either.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: Replace per-column tight_bounds with file-level tight_stats in v4 [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #18585:
URL: https://github.com/apache/iceberg/pull/18585#discussion_r4320093043
##########
format/spec.md:
##########
@@ -1011,12 +1012,35 @@ 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.
+<a id="stats-tightness"></a>
Review Comment:
Isn't this automatically generated by the heading on the next line?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org