feat!: add catalogManaged to allowed features in CREATE TABLE#2293
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2293 +/- ##
==========================================
+ Coverage 87.92% 87.94% +0.01%
==========================================
Files 160 160
Lines 50729 50848 +119
Branches 50729 50848 +119
==========================================
+ Hits 44604 44717 +113
- Misses 4364 4369 +5
- Partials 1761 1762 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
b8dddd8 to
110a256
Compare
scottsand-db
left a comment
There was a problem hiding this comment.
So, this PR is mostly fine, but why separate catalogManaged create support from this PR #2254 which adds catalogManaged -> auto enabled ICT support?
If we merge this PR, then main is broken and can create corrupt tables?
Jameson-Crate
left a comment
There was a problem hiding this comment.
+1 to Scott's point, can we maybe rearrange the stack of PRs to have #2254 at the start of the stack
|
In the latest commit, I moved some of the auto-enable ICT code from #2254 to this PR. |
## 🥞 Stacked PR Use this [link](https://github.com/delta-io/delta-kernel-rs/pull/2203/files/8cf518a944cd62f87f35cad6db205195b224c513..9c6826254bddecce6b2fe428aa9a1b6a1af1b6d2) to review incremental changes. - [stack/kernel-catalog-managed-create](#2293) [[Files changed](https://github.com/delta-io/delta-kernel-rs/pull/2293/files)] - [**stack/ccv2-create**](#2203) [[Files changed](https://github.com/delta-io/delta-kernel-rs/pull/2203/files/8cf518a944cd62f87f35cad6db205195b224c513..9c6826254bddecce6b2fe428aa9a1b6a1af1b6d2)] - [stack/ccv2-create-pt2](#2247) [[Files changed](https://github.com/delta-io/delta-kernel-rs/pull/2247/files/9c6826254bddecce6b2fe428aa9a1b6a1af1b6d2..6847bd06fd174c05674b34e4970671d5cf073d5a)] - [stack/create-table-utils-pt3](#2250) [[Files changed](https://github.com/delta-io/delta-kernel-rs/pull/2250/files/6847bd06fd174c05674b34e4970671d5cf073d5a..e1035044d9e0c419eaee5d4ebb1284adb8646bf0)] - [stack/create-table-utils-pt4](#2254) [[Files changed](https://github.com/delta-io/delta-kernel-rs/pull/2254/files/e1035044d9e0c419eaee5d4ebb1284adb8646bf0..ba618b1a85366699b2312ea3430d6adec8aefc4d)] --------- ## What changes are proposed in this pull request? Adds `create_utils` module to `delta-kernel-unity-catalog` with two public functions for building the required table properties during UC catalog-managed table creation: 1. `get_required_properties_for_disk(uc_table_id)` -- returns properties that must be written to disk in `000.json` (catalogManaged, vacuumProtocolCheck, tableId, ICT) 2. `get_final_required_properties_for_uc(snapshot, engine)` -- extracts post-commit properties to send to UC (protocol versions, feature signals, metadata config, clustering columns, ICT timestamp 3. Also adds public getters on `Snapshot` for protocol/metadata inspection: `min_reader_version`, `min_writer_version`, `reader_features`, `writer_features`, `metadata_configuration`, and `get_clustering_columns`. Other changes: - Adds `physical_to_logical_column_name` to `column_mapping.rs` for converting physical clustering column names back to logical names - Adds `CatalogManaged` to `ALLOWED_DELTA_FEATURES` in `create_table` builder - Extracts shared UC constants to `constants.rs` module ## How was this change tested? 1. `test_get_required_properties_for_disk` - verifies all 4 disk properties 2. `test_get_final_required_properties_for_uc` - round-trip test: creates table, loads snapshot, extracts UC properties, verifies protocol versions, feature signals, metadata config, ICT timestamp, and version 3. `test_get_final_required_properties_for_uc_with_clustering` - same with clustering columns, verifies JSON serialization 4. `test_public_protocol_getters` - tests Snapshot protocol getters against fixture 5. `test_metadata_configuration` - tests Snapshot metadata config getter 6. `test_catalog_managed_feature_signal_accepted` - verifies CatalogManaged in ALLOWED_DELTA_FEATURES
## 🥞 Stacked PR Use this [link](https://github.com/delta-io/delta-kernel-rs/pull/2247/files/9c6826254bddecce6b2fe428aa9a1b6a1af1b6d2..6847bd06fd174c05674b34e4970671d5cf073d5a) to review incremental changes. - [stack/kernel-catalog-managed-create](#2293) [[Files changed](https://github.com/delta-io/delta-kernel-rs/pull/2293/files)] - [stack/ccv2-create](#2203) [[Files changed](https://github.com/delta-io/delta-kernel-rs/pull/2203/files/8cf518a944cd62f87f35cad6db205195b224c513..9c6826254bddecce6b2fe428aa9a1b6a1af1b6d2)] - [**stack/ccv2-create-pt2**](#2247) [[Files changed](https://github.com/delta-io/delta-kernel-rs/pull/2247/files/9c6826254bddecce6b2fe428aa9a1b6a1af1b6d2..6847bd06fd174c05674b34e4970671d5cf073d5a)] - [stack/create-table-utils-pt3](#2250) [[Files changed](https://github.com/delta-io/delta-kernel-rs/pull/2250/files/6847bd06fd174c05674b34e4970671d5cf073d5a..e1035044d9e0c419eaee5d4ebb1284adb8646bf0)] - [stack/create-table-utils-pt4](#2254) [[Files changed](https://github.com/delta-io/delta-kernel-rs/pull/2254/files/e1035044d9e0c419eaee5d4ebb1284adb8646bf0..ba618b1a85366699b2312ea3430d6adec8aefc4d)] --------- ## What changes are proposed in this pull request? UCCommitter previously rejected version 0 commits with an unsupported error. This adds support for table creation by writing the version 0 commit file directly to the published path, bypassing the staged commit + UC ratify flow (which requires the table to already exist in UC). The connector is responsible for finalizing the table via the UC create table API after the commit succeeds. ## How was this change tested? 1. `commit_version_0_writes_published_commit`: verifies the commit file is written to the published path and exists on disk 2. `commit_version_0_conflict_when_file_exists`: verifies conflict response when the version 0 file already exists 3. existing tests pass
🥞 Stacked PR
Use this link to review incremental changes.
What changes are proposed in this pull request?
Enables creating catalog-managed Delta tables by adding
CatalogManagedtoALLOWED_DELTA_FEATURESin thecreate_tablebuilder. This allows connectors to passdelta.feature.catalogManaged = "supported"as a table property during table creation. The feature is gated behind thecatalog-managedfeature flag.How was this change tested?
test_catalog_managed_feature_signal_accepted- verifies the feature signal is accepted, removed from properties, and added to both reader and writer features