zeroshade commented on code in PR #2524:
URL: https://github.com/apache/iceberg-go/pull/2524#discussion_r4541386717
##########
table/update_schema.go:
##########
@@ -1090,11 +1090,63 @@ func (u *UpdateSchema) BuildUpdates() ([]Update,
[]Requirement, error) {
DefaultNameMappingKey: string(mappingJSON),
}))
}
+
+ updates = append(updates,
compressionPropertyUpdates(meta.props, u.schema, newSchema)...)
}
return updates, requirements, nil
}
+// compressionPropertyUpdates follows field IDs across schema changes so that
+// renames retain overrides and a deleted field's settings do not attach to a
+// new field with the same name. Collect removals before additions because a
+// renamed field may reuse the name of a deleted field.
+func compressionPropertyUpdates(props iceberg.Properties, oldSchema, newSchema
*iceberg.Schema) []Update {
Review Comment:
When the schema changes, Java handles these keys together with the other
per-column properties.
[`SchemaUpdate.applyChangesToMetadata`](https://github.com/apache/iceberg/blob/014b3ce5ffbf7c1c51dabb144f4d94e45addcd88/core/src/main/java/org/apache/iceberg/SchemaUpdate.java#L507-L529)
passes one prefix set to `PropertyUtil.applySchemaChanges`:
- `write.metadata.metrics.column.`
- `write.parquet.bloom-filter-enabled.column.`
- `write.parquet.stats-enabled.column.`
- `write.parquet.compression-codec.column.`
- `write.parquet.compression-level.column.`
As written, after a rename or delete in iceberg-go, the compression override
follows the column but `write.metadata.metrics.column.*` and
`write.parquet.bloom-filter-enabled.column.*` don't. The column silently loses
its metrics mode and bloom filter, and a deleted column's settings can attach
to a new column with the same name.
Could this become a generic `columnPropertyUpdates(props, oldSchema,
newSchema, prefixes)` called with the prefixes iceberg-go supports
(`MetricsModeColumnConfPrefix`, `ParquetBloomFilterColumnEnabledKeyPrefix` and
the two compression prefixes)? Please also add a rename/delete test for the
metrics and bloom-filter keys. Following field IDs, as you do here, also
handles renames of nested fields, which Java's exact-name match doesn't, so
keeping that approach is a plus.
--
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]
