Skip to content

feat(index): support partition-scoped global index drops - #1106

Merged
JingsongLi merged 4 commits into
apache:mainfrom
QuakeWang:index-partition-drop
Oct 15, 2026
Merged

JingsongLi merged 4 commits into
apache:mainfrom
QuakeWang:index-partition-drop

Conversation

@QuakeWang

Copy link
Copy Markdown
Member

Purpose

drop_global_index declared partitions but rejected it, while the core builder could only drop matching indexes across all partitions.

Brief change log

  • Add core partition predicates and structured SQL partition parameters.
  • Reject invalid parameters before deletion; preserve dry-run and snapshot-guarded commits.
  • Reuse Java FLOAT formatting and preserve signed-zero and NaN partition matching.

Tests

80 focused tests passed, covering partition selection, retained index definitions, invalid parameters, floating-point regressions, dry-run, and stale snapshots. CI-pinned formatting, strict Clippy, dependency and license checks passed.

API and Format

Adds GlobalIndexDropBuilder::with_partition_filter. No storage format changes.

Documentation

Document structured partitions syntax in the procedure module.

@JingsongLi JingsongLi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requirement fit: SUPPORTED. Implementation: FINDINGS at 47b42b4. Partition-scoped index maintenance has end-to-end value, but the new SQL parser can drop a different LTZ partition's index from the one requested.

Validated against the integration merge with main ae858ea: all 49 original SQL procedure tests, 17 core drop tests, 39 partition-format tests, and the stale-snapshot commit guard pass. An independent actual SQL write/build/select/selective-drop probe also preserves query results. A second actual SQL probe under TZ=Asia/Shanghai, with nonlegacy LTZ partitions, reproduces the wrong-partition deletion described inline. Please cover this supported timezone/partition-format combination before production use.

Comment thread crates/integrations/datafusion/src/procedures.rs Outdated
@QuakeWang
QuakeWang requested a review from JingsongLi October 12, 2026 09:43

@JingsongLi JingsongLi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requirement fit: SUPPORTED. Implementation: FINDINGS at b2c53bc. Partition-scoped index maintenance has end-to-end value, and the previous Shanghai LTZ issue is fixed. One narrower LTZ instant-conversion issue remains below.

Validation against the clean integration with main 87fc22e: all 50 SQL procedure tests, 17 core drop tests, 39 partition-format tests, the typed parser unit test, and the stale-snapshot commit guard pass. The LTZ suite covers Shanghai and New York in isolated processes. An additional real SQL/native range-filter probe preserves all query rows, unrelated definitions, dry-run files, and snapshot progression. A separate real SQL Parquet/write/build/$partitions/drop probe under Africa/Monrovia reproduces wrong-partition index deletion. Converting the resolved datetime to UTC before RFC3339 serialization fixes that probe; all 52 SQL tests and probes pass, and explicit-Z historical input is also verified with that correction.

Comment thread crates/integrations/datafusion/src/procedures.rs Outdated
Add a partition predicate to the core drop builder and parse structured
partition specifications in the SQL procedure. Reject invalid parameters
before deletion and preserve dry-run and snapshot-guarded commits.

Reuse Java-compatible FLOAT formatting and compare floating partition keys
with signed-zero and NaN semantics. Cover partition selection, unchanged
index definitions, invalid inputs, and non-committing previews.

Signed-off-by: QuakeWang <wangfuzheng0814@foxmail.com>
Parse non-legacy LTZ partition values in the system-local timezone while
preserving UTC semantics for legacy names and explicit timezone offsets.
Accept legacy names with omitted zero seconds and reject ambiguous or
nonexistent local times before deletion.

Cover SQL deletion in isolated timezones, timestamp precision, retained
index definitions, invalid parameters, and non-committing dry runs.

Signed-off-by: QuakeWang <wangfuzheng0814@foxmail.com>
Convert resolved LTZ values to UTC before RFC3339 serialization so
second-level timezone offsets cannot shift the partition predicate.

Cover historical Monrovia partition names and explicit UTC inputs through
SQL, including retained index definitions, dry runs, and snapshot updates.

Signed-off-by: QuakeWang <wangfuzheng0814@foxmail.com>
@QuakeWang
QuakeWang force-pushed the index-partition-drop branch from 5393dfc to 0a659a4 Compare October 13, 2026 12:40

@JingsongLi JingsongLi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requirement fit: SUPPORTED. Implementation: FINDINGS at 0a659a4. Partition-scoped index maintenance has end-to-end value. The previous Shanghai / historical-second-offset findings are fixed, including explicit-Z input. One narrower wrong-partition deletion remains below.

Validation on the clean integration with main 272cce9: 50 SQL procedure tests, 17 core drop tests, 39 partition-format tests, the typed parser test, the actual stale-snapshot guard test, and an independent native range-filter/query-preservation probe pass (109 tests). A separate actual native-schema/SQL-write/build/$partitions/drop probe reproduces the finding. Temporarily preserving the parsed timestamp without the new sub-unit truncation makes that exact probe pass. This concerns index selection; no row-data loss was observed.

Comment thread crates/integrations/datafusion/src/procedures.rs Outdated
Parse timestamp partition text directly into epoch milliseconds and nanos
instead of truncating to the declared precision or Arrow time units.
Reject nonzero fractional digits beyond nanosecond precision before deletion.

Cover native TIMESTAMP/LTZ intermediate precisions, unmatched selectors,
invalid parameters, retained index definitions, and non-committing dry runs.

Signed-off-by: QuakeWang <wangfuzheng0814@foxmail.com>

@JingsongLi JingsongLi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requirement fit: SUPPORTED. Implementation: no new actionable code finding at 3a4f6a9. The previous exact timestamp partition-identity finding is fixed, including the independent native TIMESTAMP(1) write/build/drop reproducer. The independent native range-filter probe also preserves query rows, unrelated index definitions and dry-run files.

Validation: all 51 PR SQL procedure tests plus 2 independent persisted-data probes, 17 core drop tests, 39 partition-format tests, the stale-snapshot commit guard and the timestamp-identity unit test pass on the PR head. The SQL suite includes isolated Shanghai, New York DST and historical Monrovia timezone checks. Clean integration with main 4270044 also passes the 53 SQL tests and the 17 core / 39 partition-format / stale-snapshot checks. CI-pinned formatting passes.

One nonblocking documentation follow-up: docs/src/sql.md:1448 still says “Partition filtering is not supported by this procedure.” This now contradicts the supported partitions argument. Please replace that sentence with the semicolon/comma syntax and an example, explaining that omitted keys are unrestricted and invalid specifications fail before deletion; also describe the LTZ timezone contract already documented in the procedure module. No new correctness blocker was found in this revision.

@JingsongLi
JingsongLi merged commit a17688d into apache:main Oct 15, 2026
16 checks passed
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