Repository navigation
feat(python): expose prepared sorted global index builds - #1088
Conversation
leaves12138
left a comment
There was a problem hiding this comment.
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)?), |
There was a problem hiding this comment.
[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
left a comment
There was a problem hiding this comment.
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.
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
The PyPaimon dispatcher uses only its existing single-column BTree/Bitmap capability.
Java references
Validation
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.