rdblue commented on code in PR #18405: URL: https://github.com/apache/iceberg/pull/18405#discussion_r4211976455
########## format/spec.md: ########## @@ -436,6 +436,26 @@ Two rows are the "same"---that is, the rows represent the same entity---if the i Identifier fields may be nested in structs but cannot be nested within maps or lists. Float, double, and optional fields cannot be used as identifier fields and a nested field cannot be used as an identifier field if it is nested in an optional struct, to avoid null values in identifiers. +#### Stats-only Fields + +Stats are tracked in manifests by field ID. Schema fields have assigned IDs, but additional field IDs may be assigned to track stats for derived values. For instance, lower and upper bounds for `to_lower_case(name)` are useful for case-insensitive file pruning. + +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 **`type`** that can be `partition-value` or `expr-value` +* Type-specific fields that defines 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 `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. + +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. + +The data type of a stats-only field may only change according to the type promotion rules above. Review Comment: I think that this is the right requirement as it is stated. Changes must not break existing data stored in manifest files. The other changes you suggest go further than necessary and preempt valid use cases. The implementation of a UDF can change, just like the actual expression can change without invalidating what it produces. For example, a function like `current_date()` has equivalent expressions that we see all the time, like `cast(current_timestamp() as date)`. These expressions are compatible and we don't want to specify that you must throw away the second version in order to update to the first. I think the right balance is to specify that the type cannot change in an incompatible way because that will cause a break. But for the changes you point out, we should assume that implementations won't just reuse an ID for a completely different expression. -- 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]
