Skip to content

feat(index): support dry_run in drop_global_index#489

Open
JunRuiLee wants to merge 1 commit into
apache:mainfrom
JunRuiLee:feat/global-index-drop-dry-run
Open

feat(index): support dry_run in drop_global_index#489
JunRuiLee wants to merge 1 commit into
apache:mainfrom
JunRuiLee:feat/global-index-drop-dry-run

Conversation

@JunRuiLee

@JunRuiLee JunRuiLee commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Purpose

Add dry_run to drop_global_index, matching Java's apache/paimon#8309. A dry run reports how many global index files a drop would remove, committing nothing — a safety preview before the irreversible metadata commit. Because it lives in the shared GlobalIndexDropBuilder, it covers all supported types (btree/bitmap/lumina/vindex) in one change.

Changes

  • GlobalIndexDropBuilder: new with_dry_run(bool); execute() still returns the matched count (usize), but on dry-run short-circuits before the commit, so nothing is written. The non-dry-run path is unchanged.
  • sys.drop_global_index: parses the dry_run arg (true/false, case-insensitive; invalid → error), and on dry-run returns Would drop N global index file(s) instead of committing.

Out of scope

partitions remains NotImplemented (separate follow-up).

Tests

  • Builder: dry-run returns the matched count and leaves the index entry in place (no commit), then a real drop removes it; dry-run with no match returns 0.
  • Procedure: drop_global_index(... dry_run => true) leaves $table_indexes unchanged, then a real drop empties it; invalid dry_run value is rejected.

@JunRuiLee JunRuiLee force-pushed the feat/global-index-drop-dry-run branch from fe064d2 to 73d6c02 Compare July 9, 2026 04:01
@JunRuiLee JunRuiLee marked this pull request as ready for review July 9, 2026 04:01
@JingsongLi

Copy link
Copy Markdown
Contributor

I found one correctness issue compared with the Java implementation.

Java/Flink and Java/Spark drop_global_index match the target index by the full indexed field list:

entry.indexFile().globalIndexMeta().getIndexedFieldIds().equals(indexFieldIds)

That means the primary index field plus extra_field_ids must match exactly.

In this PR, GlobalIndexDropBuilder still only matches by global_meta.index_field_id:

if global_meta.index_field_id != index_field.id() {
    continue;
}

So if there are two global index files with the same primary field but different extra_field_ids, for example [id] and [id, name], then:

CALL sys.drop_global_index(table => 'db.t', index_column => 'id', dry_run => true)

will count both files, and a real drop would delete both. Java would only match the [id] index.

I verified this locally by adding a temporary test with two BTree index files:

  • btree-id.index: index_field_id = id, extra_field_ids = None
  • btree-id-name.index: index_field_id = id, extra_field_ids = [name]

The dry run returned 2, but it should return 1.

Could you update the drop matching logic to compare the full indexed field ids, i.e. [index_field_id] + extra_field_ids, consistently with Java, and add a regression test for this case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants