Skip to content

[core] Align dedicated Blob writes and URI resolution with Java - #989

Open
JingsongLi wants to merge 2 commits into
apache:mainfrom
JingsongLi:codex/native-blob-write
Open

JingsongLi wants to merge 2 commits into
apache:mainfrom
JingsongLi:codex/native-blob-write

Conversation

@JingsongLi

@JingsongLi JingsongLi commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

Companion PyPaimon integration: apache/paimon#10305 (draft until this core change is merged).

Close the core gaps exposed by enabling scalar BLOB Arrow writes in PyPaimon Native. This covers file grouping, rolling, metadata, descriptors and URI I/O as one write/read path.

Changes

  • Finish each normal + dedicated file group together and preserve group order in commit messages. This keeps Blob row IDs aligned when normal files roll within one writer.
  • Check Blob payload size after each row, including descriptor-backed payloads. Share the data-file UUID and counter across the normal and dedicated writers, like Java's DataFilePathFactory.
  • Set data-evolution physical file sequence ranges to 0..row_count-1, honor data-evolution.write-cols-optimization.enabled, and generate indexes for the normal columns.
  • Keep completed groups owned until prepare_commit. Abort and late close/write failures remove earlier groups, external files and index sidecars.
  • Validate inline descriptor/view values before writing. Use Java's prefix parsing for schema-declared descriptors, including v1 and trailing bytes, while preserving raw-byte detection for ordinary Blob payloads.
  • Resolve known descriptors consistently in data-evolution and primary-key reads.
  • Add a shared URI reader for HTTP(S) Blob references. It uses decoded GET streams, supports gzip/deflate, verifies short reads/statuses, and reuses one response for a multi-chunk Blob copy. Ordinary paths continue using the table FileIO and its provider/credentials.

The HTTP reader follows Java's decoded-stream offsets rather than HTTP wire ranges. Unknown-length descriptors may need an additional size pass; that pass does not retain the payload in memory.

Verification

  • Reproduced the original rolling, grouping, sequence and inline-validation failures before the fixes.
  • Rust table unit tests: 1,679 passed, 3 ignored.
  • Dedicated Blob integration tests cover independent two-column rolls, null/empty payloads, external paths, indexes, optimized write columns, v1/v2 inline descriptors and primary-key reads.
  • URI tests cover redirects, chunked bodies, decoded gzip/deflate offsets, one-response sequential reads, short reads, unsupported encodings and HTTP errors.
  • PyPaimon end-to-end tests use the rebuilt binding and exercise both Python/Native planning, reading and committing, stream reuse, abort/failure cleanup and an 18 MiB HTTP Blob copied with one GET.

Final local results:

  • cargo test --locked -p paimon --lib table::: 1,679 passed, 3 ignored.
  • cargo test --locked -p paimon --lib arrow::format::blob::tests: 46 passed.
  • URI reader unit test: passed (including gzip/deflate decoded offsets).
  • Dedicated Blob and existing external-data integration suites: 12 passed.
  • cargo clippy --locked --all-targets --workspace --features fulltext,vortex -- -D warnings: passed.
  • cargo fmt --all -- --check and git diff --check: passed.
  • Expanded PyPaimon regression with all five Native options: 1,238 passed, 3 skipped, 67 subtests passed. Three Vortex cases were deselected because the optional Python Vortex dependency is absent; no Vortex behavior is changed.

@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 148b0d7, including the corresponding Java URI/error handling and row-kind filtering paths. The existing table, Blob-format, URI, dedicated/external-write, resource, row-limit, row-kind and Java-index-fixture suites passed locally (1,774 tests passed; 3 ignored). Two additional reproductions identified the issues below. The ignored-delete reproduction passes on parent fc27163 and fails on this head. Please address these before approval. The reproduction tests were run in an isolated checkout; no changes have been pushed to this PR.

Comment thread crates/paimon/src/io/uri_reader.rs Outdated

fn http_error(error: reqwest::Error) -> Error {
Error::UnexpectedError {
message: format!("HTTP Blob request failed: {error}"),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[P1] Redact signed HTTP URLs from both the error message and cause

reqwest::Error includes the request/response URL in its display output, and retaining it as source also exposes that URL when the error chain is formatted. I reproduced this through the public BlobReader::from_file_io(...).read_blobs(...) API: an unsigned /redirect descriptor redirects to /failure?signature=review-synthetic-secret, which returns HTTP 403. The returned Paimon error contains signature=review-synthetic-secret in both the message and the nested reqwest error. blob_error_with_context does not protect this case because it only replaces the original descriptor URI, not a different redirect target.

A signed URL is a credential, so ordinary HTTP failures would expose it in query/job logs. Please ensure HTTP errors and their causes never retain unredacted userinfo/query credentials, including redirected URLs, and sanitize the URI context added by the table read/write wrappers as well. Java's HttpClientUtils explicitly sanitizes HTTP failures rather than propagating the original exception. A redirect-to-signed-URL failure test should cover this path.

return Ok(None);
}
if !self.primary_key_indices.is_empty() {
super::inline_blob::validate_inline_blob_columns(

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] Apply row-kind filtering before validating inline Blob payloads

The new validation runs before enrich_rowkind_batch, which is where filter_rowkind_batch discards ignored rows. With a primary-key table configured with blob-descriptor-field=payload and ignore-delete=true, a batch containing an INSERT with a valid descriptor and a DELETE with an empty payload now fails with BlobDescriptor bytes too short, instead of dropping the DELETE and writing the INSERT. I reproduced this with explicit _VALUE_KIND values [0, 3]; the same integration test passes on parent fc27163 and fails on this head. Ignored UPDATE_BEFORE rows are subject to the same ordering issue.

Java's TableWriteImpl.writeAndReturn returns for a filtered-out row before extracting or writing its Blob value. Please validate only the rows that survive row-kind generation/filtering, still before opening any physical files, and add a mixed retained/ignored-row regression test.

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