feat(ffi): expose clustering domain metadata read path#2573
Open
chiinlquah wants to merge 2 commits into
Open
Conversation
Add `get_clustering_domain_metadata`, an FFI symbol that returns the JSON configuration of the table's `delta.clustering` domain (or NULL when absent). Mirrors `get_domain_metadata` but routes through `Snapshot::get_domain_metadata_internal`, bypassing the user-facing `delta.*`-prefix guard. Motivation: writers produce a `delta.clustering` DomainMetadata when creating a clustered table, but the existing FFI `get_domain_metadata` rejects reads on `delta.*`-prefixed domains. There's no FFI-exposed way to recover what an FFI-exposed writer wrote. External engines composing their own resolved clustering descriptors end up with a one-way wall. The internal Rust path (`Snapshot::get_physical_clustering_columns`) already does the right thing for in-process use; this just gives external engines the same access. Tests: - `test_get_clustering_domain_metadata` -- clustered table; verifies the new symbol succeeds while `get_domain_metadata` still rejects. - `test_get_clustering_domain_metadata_absent` -- unclustered table; verifies NULL return. `cargo test -p delta_kernel_ffi --lib`: 142/142 pass. `cargo clippy -p delta_kernel_ffi --all-targets -- -D warnings`: clean.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2573 +/- ##
==========================================
+ Coverage 88.66% 88.75% +0.08%
==========================================
Files 181 182 +1
Lines 61488 62184 +696
Branches 61488 62184 +696
==========================================
+ Hits 54520 55190 +670
+ Misses 4874 4854 -20
- Partials 2094 2140 +46 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Gate clustering domain read on the ClusteredTable writer feature, matching Snapshot::get_physical_clustering_columns semantics. - Centralize the constant: kernel exposes CLUSTERING_DOMAIN_NAME via the internal-api feature and a new Snapshot::get_clustering_domain_metadata method; FFI delegates to it. - Tighten the public doc comment, drop the verbatim error-string quote, and note physical-vs-logical column-name semantics. - Restructure tests as an rstest with cases for multi-column, nested path, tombstoned, latest-write-wins, and stale-domain-without-feature.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add
get_clustering_domain_metadata, an FFI symbol that returns the JSON configuration of the table'sdelta.clusteringdomain (or NULL when absent). Mirrorsget_domain_metadatabut routes throughSnapshot::get_domain_metadata_internal, bypassing the user-facingdelta.*-prefix guard.Motivation: writers produce a
delta.clusteringDomainMetadata when creating a clustered table, but the existing FFIget_domain_metadatarejects reads ondelta.*-prefixed domains. There's no FFI-exposed way to recover what an FFI-exposed writer wrote. External engines composing their own resolved clustering descriptors end up with a one-way wall.The internal Rust path (
Snapshot::get_physical_clustering_columns) already does the right thing for in-process use; this just gives external engines the same access.Tests:
test_get_clustering_domain_metadata-- clustered table; verifies the new symbol succeeds whileget_domain_metadatastill rejects.test_get_clustering_domain_metadata_absent-- unclustered table; verifies NULL return.cargo test -p delta_kernel_ffi --lib: 142/142 pass.cargo clippy -p delta_kernel_ffi --all-targets -- -D warnings: clean.What changes are proposed in this pull request?
How was this change tested?