stevenzwu commented on code in PR #18405: URL: https://github.com/apache/iceberg/pull/18405#discussion_r4212755830
########## format/spec.md: ########## @@ -442,17 +442,18 @@ Stats are tracked in manifests by field ID. Schema fields have assigned IDs, but Stats-only fields are used to track stats for derived values that are not part of the table schema and are not materialized. A stats-only field consists of: -* A **`field-id`** assigned by incrementing the table's `last-field-id` +* A **`field-id`** assigned by incrementing the table's `last-column-id` * A **`type`** that can be `partition-value` or `expr-value` -* Type-specific fields that defines how derived values are produced +* An optional **`data-type`** that determines the bound type; must be a primitive or variant +* Type-specific fields that define how derived values are produced -The `partition-value` type stores stats for the output of a partition field, identified by a `partition-field-id` type-specific field. The lower and upper bound type is the partition field's result type. +The `partition-value` type stores stats for the output of a partition field, identified by a `partition-field-id` type-specific field. The bound type is the partition field's result type and `data-type` is omitted. This may be used in v4 to filter by bucket partition values. Review Comment: > This may be used in v4 to filter by bucket partition values. do we need to mention "v4" here? the stats-only fields are allowed in v3. if v3 writer populates these partition stats fields, do we need to restrict v3 readers from using them (although they don't need to). if we keep it, maybe v4 can be renamed to v4+ for future proof? ########## format/spec.md: ########## @@ -442,17 +442,18 @@ Stats are tracked in manifests by field ID. Schema fields have assigned IDs, but Stats-only fields are used to track stats for derived values that are not part of the table schema and are not materialized. A stats-only field consists of: -* A **`field-id`** assigned by incrementing the table's `last-field-id` +* A **`field-id`** assigned by incrementing the table's `last-column-id` * A **`type`** that can be `partition-value` or `expr-value` -* Type-specific fields that defines how derived values are produced +* An optional **`data-type`** that determines the bound type; must be a primitive or variant +* Type-specific fields that define how derived values are produced -The `partition-value` type stores stats for the output of a partition field, identified by a `partition-field-id` type-specific field. The lower and upper bound type is the partition field's result type. +The `partition-value` type stores stats for the output of a partition field, identified by a `partition-field-id` type-specific field. The bound type is the partition field's result type and `data-type` is omitted. This may be used in v4 to filter by bucket partition values. -The `expr-value` type stores stats for the result of a [value expression](https://iceberg.apache.org/expressions-spec#value-expressions), stored in the `expr` field. The output type of the value expression is stored in the `data-type` field and must be a primitive or variant. +The `expr-value` type stores stats for the result of a [value expression](https://iceberg.apache.org/expressions-spec#value-expressions), stored in the `expr` field. Expressions must use only ID references. The output type of the value expression must be stored in the `data-type` field. Readers must not fail when an unsupported stats-only field `type` is found; stats for unsupported types must be ignored. -Writers must preserve existing stats for stats-only fields listed in a table's `stats-only-fields`. Writers should produce stats when possible for stats-only fields. If an expression is not supported or produces a different output type when bound, a writer should produce no stats. +Writers must preserve existing stats for stats-only fields listed in a table's `stats-fields` if the `data-type` is known (either set or specified by a supported type). Writers should produce stats when possible for stats-only fields. A writer should produce no stats by setting the field stats struct to null when an expression is not supported, produces a different output type when bound, or binding fails. Review Comment: >produces a different output type when bound nit: this reads a bit awkward to me. maybe sth like which is one of the 3 conditions for `when` ``` binding produces a different output 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
