feat(transaction): support transform-based sort orders#2765
Conversation
ReplaceSortOrderAction only supported sorting by a column's raw value: `asc`/`desc` hardcoded Transform::Identity and only accepted a column name. Java's SortOrderBuilder.asc/desc accept a Term, which can be a transform expression (bucket[N], year, truncate[W], ...). Add asc_with_transform / desc_with_transform. asc/desc are unchanged (same signature and behavior) and now delegate to the new methods with Transform::Identity, so no existing caller is affected. Transform-compatibility with the source column's type is checked at commit time via the existing SortOrder::builder().build(schema) -> check_compatibility path, matching the timing of Java's SortOrder.Builder.build() -- the spec-level type was already general (SortField.transform, check_compatibility already validate arbitrary transforms); only the transaction-layer API was narrow. Scope: this only extends the metadata-declaration API. Whether the write path sorts data to match a table's declared sort order -- for any transform, including the pre-existing identity case -- is a separate, pre-existing gap untouched here. Closes apache#2764
| )); | ||
|
|
||
| let mut action_commit = TransactionAction::commit(action, &table).await.unwrap(); | ||
| let updates = action_commit.take_updates(); |
There was a problem hiding this comment.
Can we also add an integration test that would write a metadata.json with updated sort order, and then another operation would read this json and correctly parse the transform?
There was a problem hiding this comment.
Good idea — added in eee40b8: test_sort_order_transform_survives_metadata_json_round_trip.
It commits a transform-based sort order (bucket[16] asc + truncate[4] desc) through the in-crate memory catalog, which serializes the updated table metadata to an actual metadata.json file (TableMetadata::write_to on FileIO), then reloads the table via catalog.load_table — which reads and parses that file back (TableMetadata::read_from) — and asserts both transforms and directions survive. So the transform string ("bucket[16]" etc.) is exercised through the real serialize-file-parse path, not just the in-memory TableUpdate.
…rders Per review: commit a transform-based sort order through the in-crate memory catalog (which serializes the updated metadata to a metadata.json file via TableMetadata::write_to), then reload the table (TableMetadata::read_from parses that file) and assert the transforms survive the JSON round-trip.
|
@CTTY Could you take a look at this? Thanks! |
What changes are included in this PR?
ReplaceSortOrderActiononly supported sorting by a column's raw value (an implicit identity transform) —asc/deschardcodedTransform::Identityand only accepted a column name. There was no way to declare a sort order using a transform (bucket[N],year,truncate[W], etc.), which Java'sSortOrderBuilder.asc/desc(Term, NullOrder)supports.Adds
asc_with_transform/desc_with_transform:asc/descare unchanged (same signature, same behavior) — they now delegate to the new methods withTransform::Identity, so no existing caller is affected.Whether a transform is valid for the source column's type is checked at commit time (via the existing
SortOrder::builder().build(schema)→check_compatibility), matching the timing of Java'sSortOrder.Builder.build()— an incompatible transform is only rejected once the table schema is available, not at theasc_with_transformcall site.Scope
This only extends the metadata-declaration API — which sort order (including which transform) can be recorded on a table. Whether the write path actually sorts a
RecordBatchto match a table's declared sort order is a separate, pre-existing gap: no writer incrates/iceberg/src/writer/readsSortOrderor sorts data today, for any sort order, including the pre-existing identity case. This PR does not touch that; it only makes a wider set of sort orders declarable.Are these changes tested?
test_replace_sort_order_with_transform— the builder records the requested transform per field.test_replace_sort_order_with_transform_commits— end-to-end commit; the resultingTableUpdate::AddSortOrder'sSortFieldcarries the requested transform.test_replace_sort_order_rejects_incompatible_transform—Transform::Yearon alongcolumn is rejected at commit time withErrorKind::Unexpected, exercising the existingcheck_compatibilitypath.test_replace_sort_order(plainasc/desc) passes unchanged.Full
iceberglib suite passes; clippy and rustfmt clean.