Skip to content

feat(python): expose prepared sorted global index builds - #1088

Merged
JingsongLi merged 2 commits into
apache:mainfrom
JingsongLi:codex/native-index-build-followup
Oct 10, 2026
Merged

JingsongLi merged 2 commits into
apache:mainfrom
JingsongLi:codex/native-index-build-followup

Conversation

@JingsongLi

@JingsongLi JingsongLi commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

PyPaimon counterpart: apache/paimon#10482. Its CI continues using Rust main, so this core PR should merge first.

PyPaimon already builds local BTree and Bitmap global indexes, but cannot use the core Rust builder while retaining its build-then-commit API. Expose that existing capability through a thin Python wrapper and make prepared indexes safe to publish with a regular TableCommit.

Changes

  • Add SortedGlobalIndexBuildBuilder.build() returning unpublished CommitMessages; retain execute() with its latest-snapshot guard.
  • Support partition-only predicates using the shared partition validation and retain the table's REST/branch snapshot manager.
  • Use table-configured global-index external paths.
  • Align sorted-index.records-per-file, its fallback keys and the 25,000,000 default with Java SortedIndexOptions.
  • Write the Java DataEvolutionIndexSourceMeta (DEIX/version 1) source snapshot. On every commit attempt, validate current row coverage and target-column changes since that snapshot, including historical DELETEs and same-commit updates. Append-only changes and unrelated columns remain allowed. Returned files are retained on failure; explicit abort remains available.
  • Normalize nullable IN/NOT IN literals in core and accept them in the Python predicate bridge, including Java's empty-set NULL behavior.
  • Add the builder binding, Table factory and matching datafusion.pyi declarations.

The PyPaimon dispatcher uses only its existing single-column BTree/Bitmap capability.

Java references

  • SortedIndexOptions
  • SortedGlobalIndexWriter / DataEvolutionIndexSourceMeta
  • DataEvolutionConflictDetection global-index row-ID existence checks
  • In / NotIn predicate evaluators

Validation

  • cargo test -p paimon --lib sorted: 91 passed, 1 ignored
  • cargo test -p paimon --lib table::table_commit: 118 passed
  • cargo test -p paimon --lib spec::predicate: 106 passed
  • cargo test -p paimon --lib data_evolution_index_source: 1 passed
  • cargo +1.98.0 clippy -p paimon -p pypaimon_rust --all-targets -- -D warnings
  • PyPaimon REST/native and existing regressions: 283 passed, 32 subtests; 104 native plans exercised
  • Final native index integration module: 87 passed

Regression cases cover unpublished files, explicit abort, partition/incremental builds, REST snapshot selection/errors, external paths, scalar key types, both committers, uncertain publication outcomes, source changes/deletions, and equivalent empty partition encodings.

@leaves12138 leaves12138 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reviewed this head together with apache/paimon#10482 at f673605e2bafde83f07f8e04b75a5acf738cdaa7. One P2 remains: the existing Python binding regression still expects empty IN to be rejected, so the normal binding test target fails. Accepting empty IN is the intended Java-aligned behavior; please update the test rather than restore the old restriction. See the inline comment.

No additional blocking runtime issue was found in the prepared-index build/commit path. I checked the build-versus-execute API, REST/branch snapshot selection, configured external paths, Java-compatible source metadata and option precedence, authorization/capability gates, per-attempt conflict validation, file retention after unsuccessful or uncertain publication, and nullable set predicates.

Independent verification against the rebuilt extension:

  • Rust core library: 3,912 passed, 6 ignored.
  • Selected binding read/table/resolved-table/commit/write/update/vector-search suites: 298 passed, 1 failed (the stale empty-IN assertion). The failing test also reproduces alone.
  • Affected PyPaimon REST/index/commit suites: 283 passed plus 32 subtests.
  • Related Native suites: 997 passed plus 96 subtests.
  • Reviewer-added cases: 16 passed: eight publication-conflict retries across BTree/Bitmap and both committers, plus eight Native empty/nullable IN/NOT IN read cases. Target-column updates during the first publication attempt are rejected on retry; unrelated-column updates can retry and publish.
  • Rust formatting, configured Flake8 for the six changed Python files, and Python 3.6 syntax checks passed.

Java compatibility was checked against the Java source implementation; I did not run a separate Java process or test against a production service.

}
pb.is_not_in(&field, ds)
}
"in" => pb.is_in_with_nulls(&field, to_nullable_datums(literals_obj)?),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[P2] Update the binding regression for accepted empty IN lists

This correctly allows an empty IN list, but bindings/python/tests/test_read.py:1055 still wraps with_filter({"method": "in", "field": "id", "literals": []}) in pytest.raises(ValueError). With the extension rebuilt from this head, python -m pytest tests/test_read.py::test_filter_wrong_literal_count_raises deterministically fails with DID NOT RAISE ValueError; this is also the only failure in the selected binding suites (298 passed, 1 failed). The normal Python integration target runs this test via make install && make test, so the stale assertion blocks that target.

Please update the old test while retaining the invalid-arity check for equal, and add positive binding read assertions for empty IN and empty NOT IN on nullable data. Keep the new production behavior: Java IN with an empty set matches no rows, and NOT IN with an empty set matches only non-NULL rows. Restoring the previous empty-list rejection would break the intended Java parity.

@leaves12138 leaves12138 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Re-reviewed this head together with unchanged apache/paimon#10482 at f673605e2bafde83f07f8e04b75a5acf738cdaa7. The previous P2 is fixed, and no new blocking issue was found.

The follow-up only changes bindings/python/tests/test_read.py. It removes the stale empty-IN rejection, retains invalid-arity checks for equal with both zero and multiple literals, and adds eight read-result regressions covering empty sets, NULL-only sets, mixed NULL/non-NULL sets and ordinary sets on nullable data. The expected IN/NOT IN results agree with the Java predicate implementations; importantly, the correct production acceptance behavior has not been reverted.

Current verification:

  • Focused fix/Java-semantic tests: 10 passed.
  • Complete Python binding suite: 417 passed.
  • Affected PyPaimon REST/index/commit suites: 283 passed plus 32 subtests.
  • Reviewer-added commit-retry and Native set-predicate cases: 16 passed.
  • Changed-test syntax check passed. The current Python integration check is also successful.

I verified file-by-file that all production Rust code and Python binding wrappers are byte-identical to the previous reviewed head, so these tests reuse the extension rebuilt for that head. The full binding run used PyAV and a locally provisioned Native SQLContext smoke-test warehouse, not the Java Docker warehouse. Java compatibility is a source-level semantic check; no separate Java process or production-service integration was run.

My existing approval of #10482 still stands. Its Native CI builds Rust main, so the corrected Rust counterpart should land on main before merging that dependent Python PR.

@JingsongLi
JingsongLi merged commit d04eb4b into apache:main Oct 10, 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