You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "JunRuiLee (via GitHub)" <gi...@apache.org> on 2026/07/08 13:11:47 UTC

[PR] feat(index): support lumina and vindex global index drop [paimon-rust]

JunRuiLee opened a new pull request, #487:
URL: https://github.com/apache/paimon-rust/pull/487

   ## Purpose
   
   The Rust side can **build** lumina and vindex (`ivf-*`) global indexes but cannot **drop** them — `drop_global_index` (procedure + builder) was restricted to the sorted types `btree`/`bitmap`. This closes that gap, matching the generic drop already supported by Java (`DropGlobalIndexProcedure`, no type allowlist) and pypaimon.
   
   ## Changes
   
   - **New shared helper** `normalize_global_index_type_for_drop` (in `global_index_types.rs`, re-exported from `paimon::table`): canonicalizes any supported global index type case-insensitively. Both lumina aliases (`lumina`, legacy `lumina-vector-ann`) canonicalize to `lumina`; each vindex type keeps its own identity (`ivf-flat` ≠ `ivf-pq`). One shared `SUPPORTED_GLOBAL_INDEX_TYPES_FOR_DROP` constant keeps the builder and procedure error messages in sync.
   - **Generalized the drop builder** `BTreeGlobalIndexDropBuilder` → `GlobalIndexDropBuilder` (accessor `Table::new_global_index_drop_builder`). Matching now normalizes **both** the request and each stored entry's type, so a `lumina` request matches a stored legacy `lumina-vector-ann` entry and case variants, while a specific vindex type never matches a different one on the same column. Drop stays **metadata-only** (marks index-manifest entries `FileKind::Delete`); physical files are left to orphan/expire, unchanged from btree/bitmap.
   - **DataFusion procedure** `sys.drop_global_index` now accepts `btree`/`bitmap`/`lumina`/vindex types via the shared helper.
   
   ## Out of scope
   
   `dry_run` and `partitions` remain `NotImplemented` (as they already were for btree/bitmap).
   
   ## Tests
   
   - Helper unit tests: case-insensitivity, lumina alias canonicalization, vindex identity, unsupported → None.
   - Builder tests: `lumina` drops a stored legacy `lumina-vector-ann` entry; uppercase `IVF-FLAT` drops only `ivf-flat` (leaving `ivf-pq`/`lumina`/`btree`); unsupported-type error; existing btree/bitmap coverage retained.
   - Procedure tests: `drop_global_index` accepts `lumina` and `ivf-pq`.


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(index): support lumina and vindex global index drop [paimon-rust]

Posted by "JunRuiLee (via GitHub)" <gi...@apache.org>.
JunRuiLee commented on code in PR #487:
URL: https://github.com/apache/paimon-rust/pull/487#discussion_r3544767811


##########
crates/paimon/src/table/mod.rs:
##########
@@ -219,8 +222,8 @@ impl Table {
         BTreeGlobalIndexBuildBuilder::new(self)
     }
 
-    pub fn new_btree_global_index_drop_builder(&self) -> BTreeGlobalIndexDropBuilder<'_> {
-        BTreeGlobalIndexDropBuilder::new(self)
+    pub fn new_global_index_drop_builder(&self) -> GlobalIndexDropBuilder<'_> {

Review Comment:
   `new_btree_global_index_drop_builder` and `BTreeGlobalIndexDropBuilder` were introduced in #472, which merged after the v0.2.0 release and isn't part of any published crates.io release — the latest published `paimon` is 0.2.0. So this API has never shipped in a release, and no downstream crate can depend on it; renaming it here isn't a breaking change against any released version, only a change to unreleased, main-only API.
   
   Given that, I'd prefer the clean rename over a deprecated forwarding method + alias, which would be permanent shims for a compatibility guarantee we don't owe pre-release. 



-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(index): support lumina and vindex global index drop [paimon-rust]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi merged PR #487:
URL: https://github.com/apache/paimon-rust/pull/487


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(index): support lumina and vindex global index drop [paimon-rust]

Posted by "QuakeWang (via GitHub)" <gi...@apache.org>.
QuakeWang commented on code in PR #487:
URL: https://github.com/apache/paimon-rust/pull/487#discussion_r3544796433


##########
crates/paimon/src/table/mod.rs:
##########
@@ -219,8 +222,8 @@ impl Table {
         BTreeGlobalIndexBuildBuilder::new(self)
     }
 
-    pub fn new_btree_global_index_drop_builder(&self) -> BTreeGlobalIndexDropBuilder<'_> {
-        BTreeGlobalIndexDropBuilder::new(self)
+    pub fn new_global_index_drop_builder(&self) -> GlobalIndexDropBuilder<'_> {

Review Comment:
   That makes sense, thanks for clarifying. I missed that this API has not been included in any published release yet. In that case I agree this is not a breaking change against released users, and the clean rename is preferable here.



-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(index): support lumina and vindex global index drop [paimon-rust]

Posted by "JunRuiLee (via GitHub)" <gi...@apache.org>.
JunRuiLee commented on code in PR #487:
URL: https://github.com/apache/paimon-rust/pull/487#discussion_r3544767811


##########
crates/paimon/src/table/mod.rs:
##########
@@ -219,8 +222,8 @@ impl Table {
         BTreeGlobalIndexBuildBuilder::new(self)
     }
 
-    pub fn new_btree_global_index_drop_builder(&self) -> BTreeGlobalIndexDropBuilder<'_> {
-        BTreeGlobalIndexDropBuilder::new(self)
+    pub fn new_global_index_drop_builder(&self) -> GlobalIndexDropBuilder<'_> {

Review Comment:
   `new_btree_global_index_drop_builder` and `BTreeGlobalIndexDropBuilder` were introduced in #472, which merged after the v0.2.0 release and isn't part of any published crates.io release — the latest published `paimon` is 0.2.0. So this API has never shipped in a release, and no downstream crate can depend on it; renaming it here isn't a breaking change against any released version, only a change to unreleased, main-only API.
   
   Given that, I'd prefer the clean rename over a deprecated forwarding method + alias, which would be permanent shims for a compatibility guarantee we don't owe pre-release. If the project would rather keep deprecated aliases as policy even for unreleased APIs, I'm happy to add them — just let me know.



-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(index): support lumina and vindex global index drop [paimon-rust]

Posted by "QuakeWang (via GitHub)" <gi...@apache.org>.
QuakeWang commented on code in PR #487:
URL: https://github.com/apache/paimon-rust/pull/487#discussion_r3544696037


##########
crates/paimon/src/table/mod.rs:
##########
@@ -219,8 +222,8 @@ impl Table {
         BTreeGlobalIndexBuildBuilder::new(self)
     }
 
-    pub fn new_btree_global_index_drop_builder(&self) -> BTreeGlobalIndexDropBuilder<'_> {
-        BTreeGlobalIndexDropBuilder::new(self)
+    pub fn new_global_index_drop_builder(&self) -> GlobalIndexDropBuilder<'_> {

Review Comment:
   This renames the public drop builder API and removes the existing `Table::new_btree_global_index_drop_builder()` entry point. Since `paimon::table` is public, downstream users calling the old API will fail to compile. Could we keep a deprecated forwarding method, and optionally a deprecated `BTreeGlobalIndexDropBuilder` type alias to `GlobalIndexDropBuilder`, while introducing the generalized API?



-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org