Skip to content

[python] Use native scans for supplied primary-key index results - #10591

Merged
JingsongLi merged 3 commits into
apache:masterfrom
JingsongLi:codex/native-index-result-scan
Oct 15, 2026
Merged

JingsongLi merged 3 commits into
apache:masterfrom
JingsongLi:codex/native-index-result-scan

Conversation

@JingsongLi

@JingsongLi JingsongLi commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

Use Rust core for PyPaimon's existing supplied primary-key index-result scans. PK vector/full-text results already carry a snapshot and physical file positions; these must be handed to the normal scan without resolving latest again or changing their source splits.

Depends on apache/paimon-rust#1133. This PR is draft until that Rust API is available on main. Native CI continues building Rust main.

  • Bridge existing PK split results and DE row-ID results to core GlobalIndexResult.
  • Consume the shared Native GlobalIndexResult returned by searches directly in Native scans, avoiding explicit result conversion or a Python split-wire round trip. Exposing mutable splits or copying/serializing a result invalidates the original native handle, so subsequent scans observe Python metadata edits.
  • Keep snapshot 0 for empty PK results and retain snapshot/file/DV/range/score metadata.
  • Let core align DE scores after pruning/distribution and preserve incremental windows.
  • Match Java under query authorization: ignore supplied raw-index selections in both Python and Native paths.
  • Assert Java float32 split-score values for Native plans.

Tests

Locally built the binding from the dependent Rust PR.

  • 132 passed, including result-scan tests, planner unit tests, expanded planning tests, and all four search families sharing one Native result type; 46 subtests passed.
  • 188 existing PK full-text/vector/hybrid search tests passed, including Java-generated PK archives.
  • Related full-text, hybrid, PK scalar-index, authorization, distribution, and incremental suites were also exercised. The combined run found two double-vs-float32 score assertions; both now assert Java float32 wire values and passed the focused rerun.
  • 188 Rust Python-binding vector/read/resolved-table tests passed.
  • Flake8, Python 3.6 syntax parsing, license headers, and diff whitespace checks passed.

Regression cases cover Native/Python readers, historical and empty results, source metadata preservation, filter/limit/shard ordering, direct Core result handoff for vector/batch-vector/full-text/hybrid, ranked reads with final split-score checks, pickle and shallow-copy aliasing, edited Python selections, DE score ordering and incremental windows, and ignoring raw index results under REST authorization.

@leaves12138 leaves12138 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.

Reviewed the native GlobalIndexResult handoff against the Java scan semantics and the dependent Rust API. The PK snapshot/physical-split path, DE row-ID score propagation, incremental handling, query-auth fallback, and Python metadata invalidation are consistent. Focused planner tests and static checks passed; no blocking issues found.

@JingsongLi
JingsongLi marked this pull request as ready for review October 15, 2026 15:00

@leaves12138 leaves12138 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.

Blocking: the current commit fails the existing Python test suite on both Python 3.12 and 3.13. The failure is : receives instead of . The regression comes from , where the new guard treats the test's minimal as truthy and skips . Please make this check robust for the existing scanner contract or update the fixture/coverage so the full Python CI passes.

@leaves12138

Copy link
Copy Markdown
Contributor

Correction to the review body above (the shell stripped inline backticks):

Blocking: the current commit fails the existing Python test suite on both Python 3.12 and 3.13. The failure is pypaimon/tests/global_index_test.py::GlobalIndexScalarFallbackTest::test_split_planning_merges_indexed_and_unindexed_ranges: DataEvolutionSplitGenerator receives row_ranges=None instead of [Range(1, 1), Range(10, 10 ** 12)].

The regression comes from paimon-python/pypaimon/read/scanner/file_scanner.py:477, where the new and not self.table.options.query_auth_enabled guard treats the test's minimal Mock().options.query_auth_enabled as truthy and skips _eval_global_index. Please make this check robust for the existing scanner contract or update the fixture/coverage so the full Python CI passes.

@leaves12138 leaves12138 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.

Correction to the review body above (the shell stripped inline backticks):

Blocking: the current commit fails the existing Python test suite on both Python 3.12 and 3.13. The failure is pypaimon/tests/global_index_test.py::GlobalIndexScalarFallbackTest::test_split_planning_merges_indexed_and_unindexed_ranges: DataEvolutionSplitGenerator receives row_ranges=None instead of [Range(1, 1), Range(10, 10 ** 12)].

The regression comes from paimon-python/pypaimon/read/scanner/file_scanner.py:477, where the new and not self.table.options.query_auth_enabled guard treats the test's minimal Mock().options.query_auth_enabled as truthy and skips _eval_global_index. Please make this check robust for the existing scanner contract or update the fixture/coverage so the full Python CI passes.

@leaves12138 leaves12138 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.

Re-reviewed the updated HEAD. The query-auth test now explicitly covers both enabled and disabled options, and the focused regression test plus the complete global_index_test.py suite pass locally. The native GlobalIndexResult handoff remains consistent with Java semantics and the dependent Rust API. No blocking issues found.

@JingsongLi
JingsongLi merged commit c9a36a8 into apache:master Oct 15, 2026
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