rdblue commented on code in PR #18405: URL: https://github.com/apache/iceberg/pull/18405#discussion_r4365070228
########## format/spec.md: ########## @@ -474,6 +474,27 @@ 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-column-id` +* A **`type`** that can be `partition-value` or `expr-value` +* 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 bound type is the partition field's result type and `data-type` is omitted. This may be used to store bucket partition values in stats for filtering. + +The `expr-value` type stores stats for the result of a [value expression](expressions-spec.md#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-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 if an expression is not supported, produces an incompatible output type when bound, or binding fails. Review Comment: This is a good point, although I don't think that it affects the changes in this PR. This problem with `date` to `timestamp` type promotion was introduced when we added that promotion in v3 and is more general than stats. In v3, if you are storing data bucketed by a date field, then data files have the bucket in their `partition` field. Promoting the type invalidates that partition value because the timestamp's bucket may not match. I was careful to avoid this in the original type promotion rules (which is why int is promoted to long before bucketing) but missed this. I think the right solution is a separate PR that disallows `date` to `timestamp` type promotion when there is a bucket partition field on the column. I think maybe a general prohibition on this would also be appropriate, which is why this will be a good separate discussion. The remaining question is whether this PR should make a rule prohibiting type promotion for the references used in stats-only fields with `type=expr-value`. There are examples where type promotion _can_ produce different values, like `size_of(some_col)` that would change from 4 to 8 when promoting an `int` to a `long`. I could probably be convinced otherwise, but I think that this is a risk we can't really eliminate for expressions. Expressions can change, either by the definition of a UDF being updated, a built-in changing behavior, or by a user changing the expression entirely. I don't think that we want to be heavy-handed in any of those cases and require that these expressions can't evolve at all, nor would we be able to expect that to happen if we did (because UDFs are user-defined). We should provide guidance that you should only use expressions that produce consistent output across type promotions and should not modify expressions in a way that breaks the existing data. I'm not sure what we can do beyond that. -- 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]
