Skip to content

feat: expand Python native reads with nested projection and row kinds - #887

Merged
JingsongLi merged 2 commits into
apache:mainfrom
JingsongLi:codex/native-coverage-expansion
Sep 20, 2026
Merged

JingsongLi merged 2 commits into
apache:mainfrom
JingsongLi:codex/native-coverage-expansion

Conversation

@JingsongLi

@JingsongLi JingsongLi commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Purpose

Expand the Python native reader beyond top-level snapshot reads so PyPaimon can use Rust for nested projections and changelog-aware reads.

Changes

  • add nested ROW projection and literal MAP-key projection to the Python binding
  • expose row-kind-aware reads for snapshot and streaming splits
  • preserve physical +I/-U/+U/-D events for streaming primary-key splits
  • prune shared-shredding Parquet MAP reads to the physical columns that may contain selected keys
  • assemble selected-key MAP results from partial physical structs, including missing keys and overflow
  • keep complete MAP reads when field/default aggregation requires full merge input

The MAP I/O regression test compares selected-key and full MAP reads over 100,000 rows and requires selected-key reads to use at least 20% fewer data bytes.

Tests

  • cargo test -p pypaimon_rust nested_projection_
  • cargo test -p paimon test_parquet_map_shredding_selected_key
  • cargo test -p paimon --test audit_log_table_test
  • cargo clippy -p paimon -p pypaimon_rust --all-targets -- -D warnings

Paired Python integration: apache/paimon#10009.

@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#10009 using a freshly built native extension. The 2,931 core library tests, 20 audit-log tests, two nested-projection binding tests, and two selected-key Parquet tests pass. An additional end-to-end comparison found one selected-key decoding issue below.

let Some(encoded) = description.strip_prefix(SELECTED_KEYS_PREFIX) else {
return Ok(None);
};
if encoded.is_empty() {

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] Preserve the valid empty-string MAP key

An empty encoded suffix does not necessarily mean an empty key list: project_nested_field encodes the valid selection [""] as __PAIMON_MAP_SELECTED_KEYS:. PyPaimon's map_selected_keys_field accepts this selection, and map_selected_keys decodes the suffix to [""], but this check rejects it.

I reproduced this against the real extension with an append table containing attrs: MAP<STRING, INT>, fields.attrs.map.storage-layout=shared-shredding, and a row {'attrs': {'': 10, 'other': 11}}. with_projection(["attrs['']"]) returns 10 on the Python reader (and works on ordinary non-shredded MAP files), but the native shared-shredding reader raises Selected-key MAP field 'attrs' has no keys during batch iteration. This is after the native reader has been returned, so it does not fall back to Python.

Please decode this as the single empty-string key to match the existing convention, or retain the full-MAP fallback for this key. A shared-shredding regression test selecting only attrs[''] would cover the failure.

@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 the update. The empty-string MAP-key issue is fixed, and the added shared-shredding regression test covers the previously rejected encoding. I rebuilt the native extension from this head and reran the original end-to-end Python/native comparison: it now passes. The two selected-key Parquet tests, 2,931 core library tests (two ignored), cargo fmt check, and all 172 targeted Python tests with the paired apache/paimon#10009 update also pass. No remaining blocking findings. LGTM.

@JingsongLi
JingsongLi merged commit 30c171d into apache:main Sep 20, 2026
14 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