[python] BLOB descriptor parsing and read-path compatibility. by Stephen0421 · Pull Request #9148 · apache/paimon

Stephen0421 · 2026-08-10T10:12:54Z

Summary

First PR in the stacked series for #9099. Shared descriptor-byte parsing and read/write foundations for managed BLOB v1/v2 compatibility — no PK-specific logic.

Parsing API (blob.py): from_bytes (v2 magic heuristic) vs from_descriptor_bytes (v1 strict + v2 deserialize with optional trailing padding)
Batch read: BlobInlineConvertReader uses from_descriptor_bytes for descriptor fields
Row read: descriptor_field_indices when blob-as-descriptor=true → OffsetRow.get_blob() uses from_descriptor_bytes
Config: legacy blob.stored-descriptor-fields; blank blob-descriptor-field treated as unset
Write: blob_format_writer rejects truncated copies (EOFError) when descriptor length is known
URI lifecycle: UriReaderFactory owned FileIO tracking; clear_cache() without LRU double-close; FileIO close() wiring

Behavior changes (intentional)

Descriptor columns: malformed bytes → ValueError (was silent BlobData passthrough)
blob-descriptor-field="" + legacy set → now falls back to blob.stored-descriptor-fields
from_bytes on arbitrary inline payload → unchanged (BlobData; v2-only heuristic)

Test plan

BlobTest
UriReaderFactoryTest
CI green

Follow-ups

PR2: managed BLOB lifecycle + staged commit
PR3: PK managed BLOB write
PR4: PK managed BLOB read/view
PR5: dynamic bucket HASH callback

Related: #9099

JingsongLi

Inline comments focus on behavioral regressions and consistency with the Java implementation.

JingsongLi · 2026-08-10T13:30:04Z

            data = blob_value.to_data()
            crc32 = self._write_with_crc(data, crc32)
        else:
+            expected_length = self._expected_blob_length(blob_value)


Please restrict the exact-length path to an exact BlobRef (for example, type(blob_value) is BlobRef). This currently trusts to_descriptor().length for every Blob subtype, so a custom subtype whose new_input_stream() exposes more bytes than its descriptor is silently truncated. I reproduced a descriptor length of 3 with a stream containing abcdef; this branch writes only abc, while the base branch writes all 6 bytes. The Java writer deliberately checks blob.getClass() == BlobRef.class and reads other implementations to EOF.

Thanks for catching this. Agreed — the exact-length copy path should be restricted to plain BlobRef, matching Java's blob.getClass() == BlobRef.class check.

Updated to use type(blob_value) is BlobRef before calling _copy_exactly. Other Blob subtypes (e.g. resolved BlobView) now read to EOF as before. Removed the test that expected truncation on resolved BlobView, since that behavior was incorrect.

JingsongLi · 2026-08-10T13:30:04Z

+        value = self.options.get(CoreOptions.BLOB_DESCRIPTOR_FIELD, None)
+        if isinstance(value, str):
+            value = value.strip()
+        if not value:


Please choose the legacy fallback based on key presence rather than value truthiness. Java Options.applyWithOption consults fallback keys only when the canonical key is absent. With blob-descriptor-field="" and blob.stored-descriptor-fields set, Java resolves an empty descriptor-field set, but this code revives the legacy fields. That can make Python interpret ordinary BLOB bytes as descriptors or choose a different write layout. An explicitly present blank canonical value should win.

Good point. The legacy fallback should only apply when the canonical key is absent, not when it is explicitly set to a blank value.

Updated blob_descriptor_fields() to consult blob.stored-descriptor-fields only when blob-descriptor-field is None, matching Java Options.applyWithOption / parseCommaSeparatedSet semantics. With blob-descriptor-field="" and legacy set, Python now resolves an empty descriptor-field set, same as Java.

JingsongLi · 2026-08-10T13:30:04Z

+                return None
+            uri_length = struct.unpack('<I', raw[offset:offset + 4])[0]
+            total = offset + 4 + uri_length + 16
+            if total != len(raw):


from_descriptor_bytes is used when the schema/storage context already says that the value is a descriptor, but this exact-length check rejects v1 bytes with trailing padding. Java BlobDescriptor.deserialize accepts trailing bytes for every supported version, so a padded legacy v1 descriptor can be read by Java but is rejected by Python (and the new test currently codifies that mismatch). Please deserialize v1 with the same Java semantics here, or coordinate a strict contract change on both implementations.

Agreed — from_descriptor_bytes should follow Java BlobDescriptor.deserialize semantics and accept trailing padding for both v1 and v2.

Switched to always calling BlobDescriptor.deserialize() in from_descriptor_bytes, and updated the test to verify padded v1 descriptors are accepted. The heuristic from_bytes entry point still uses v2 magic only (is_blob_descriptor) so inline v1-shaped payload bytes are not misclassified.

Also aligned from_bytes(allow_blob_data=False) with Java Blob.fromBytes: when allow_blob_data=False, any bytes are deserialized as a descriptor (including v1), not only v2 magic-prefixed ones.

JingsongLi

One remaining Java-compatibility issue in the legacy v1 BLOB view read path.

JingsongLi · 2026-08-11T09:50:00Z

+
+    if not CoreOptions.blob_as_descriptor(table.options):
+        return set()
+    return descriptor_field_indices(


[P2] When blob-as-descriptor=true, BlobInlineConvertReader resolves configured blob-view-field values to descriptor.serialize() before rows are exposed. If the upstream value is a legacy v1 descriptor, Python reserializes it as v1 without the magic header (unlike Java, whose BlobDescriptor.serialize() always writes CURRENT_VERSION). Because this helper marks only blob-descriptor-field, OffsetRow.get_blob() takes the heuristic path for the resolved view column and returns BlobData containing the descriptor bytes instead of a reference to the payload. Please either mark resolved view fields as descriptor-backed here or normalize reserialized descriptors to v2, and add a v1 blob-view regression test.

Thanks for catching this. Fixed in the latest push:

When blob-as-descriptor=true, resolved blob-view-field columns are now included in descriptor routing, so OffsetRow.get_blob() returns BlobRef instead of BlobData.

BlobDescriptor.serialize() now always writes v2 + magic (aligned with Java), so v1 descriptors re-serialized by BlobInlineConvertReader are unambiguous.

Added test_offset_row_get_blob_v1_resolved_blob_view_field plus routing/serialize coverage.

JingsongLi

Four inline comments for the confirmed correctness and error-handling regressions found in the latest PR head.

JingsongLi · 2026-08-13T02:14:38Z

+        prescan_reader_factory=lambda names: split_read._create_blob_view_prescan_reader(names),
+        blob_parallelism=split_read._blob_parallelism,
+    )
+    return BatchToRecordReaderAdapter(batch_reader)


[P1] Please preserve the typed field metadata when converting this batch reader back to rows. BatchToRecordReaderAdapter._ArrowBatchIterator currently creates each OffsetRow without file_io, BLOB/descriptor/vector indices, or the view lookup. On this new MergeFileSplitRead bridge, a valid descriptor/view BLOB is materialized, but to_iterator() consumers then get TypeError: Field ... is not a BLOB field from row.get_blob(pos). This is reachable for Java-created primary-key BLOB tables. Please propagate the converted reader metadata into every OffsetRow and add a merge-read to_iterator() + get_blob() regression test.

Thanks. BatchToRecordReaderAdapter now copies file_io, BLOB/descriptor/vector indices, and the view lookup into every OffsetRow. wrap_record_reader_with_blob_inline_convert also sets that metadata on the batch adapter. Added wrap + adapter roundtrip coverage, including merge to_iterator() + get_blob().

JingsongLi · 2026-08-13T02:14:38Z

+        value_fields = self.read_fields[-self.value_arity:]
+        prescan_fields = [f for f in value_fields if f.name in field_names]
+        if not prescan_fields:
+            return EmptyFileRecordReader()


[P1] Please return an EmptyRecordBatchReader here. BlobInlineConvertReader unconditionally calls read_arrow_batch() on its prescan reader, while EmptyFileRecordReader only implements read_batch(). When a BLOB-view table is read through this merge path with a projection that excludes every configured view field, prescan_fields is empty and the query fails because the returned reader has no read_arrow_batch() method. Please add a projection regression test for that case.

Fixed. An empty view projection now returns EmptyRecordBatchReader. A non-empty merge prescan wraps the row reader with RecordReaderToBatchAdapter so BlobInlineConvertReader can call read_arrow_batch(). Added projection tests, including convert + empty prescan reading the main batch.

JingsongLi · 2026-08-13T02:14:38Z

+        try:
+            # Accept v1/v2 descriptors with trailing padding (Java deserialize).
+            return Blob.from_descriptor_bytes(raw, self._file_io)
+        except ValueError:


[P2] Please do not downgrade descriptor parsing failures to BlobData here. Membership in descriptor_field_indices already establishes that the schema/storage context requires a descriptor. With this fallback, a truncated v1 descriptor or corrupted/future v2 descriptor is silently returned as raw payload by the row API, while BlobInlineConvertReader raises ValueError for the same bytes. This contradicts the PR stated fail-fast behavior and makes row and batch reads inconsistent. Materialized payloads are already represented by clearing the descriptor indices in BlobInlineConvertReader, so descriptor-indexed values should use from_descriptor_bytes strictly.

Agreed. Descriptor-indexed values now use from_descriptor_bytes strictly; truncated or corrupt bytes raise ValueError instead of falling back to BlobData. Materialized payloads are represented by clearing descriptor_field_indices in BlobInlineConvertReader when blob-as-descriptor=false. Added a truncated-v1 rejection test.

JingsongLi · 2026-08-13T02:14:38Z

+                        crc32 = self._write_with_crc(chunk, crc32)
+                        chunk = stream.read(self.copy_buffer_size)
            finally:
                stream.close()


[P2] Please preserve the truncation EOFError when closing the source stream also fails. If a known-length descriptor expects 10 bytes, the source ends after 3, and a remote/native stream close() raises, this finally block replaces the useful EOFError with the close exception; OffsetInputStream can then attempt a second close during destruction. Please retain the copy/read exception while best-effort closing, and surface the close error only when copying succeeded. A short, close-failing source test would cover this failure path.

Fixed. _write_blob_data now keeps the copy/read exception (including EOFError) and only surfaces a close() failure if the copy succeeded. Added tests for close-failing truncated copy and close-failing successful copy.

Introduce explicit descriptor-byte parsing for managed BLOB v1/v2 reads, legacy blob.stored-descriptor-fields fallback, write-path truncation checks, UriReaderFactory lifecycle handling, and row-level descriptor field routing for blob-as-descriptor tables.

Stephen0421 mentioned this pull request Aug 10, 2026

[python] Support BLOBs in primary-key tables. #9099

Draft

JingsongLi reviewed Aug 10, 2026

View reviewed changes

Stephen0421 force-pushed the pypaimon-blob-pr1-foundation branch from f20aa56 to 0c0865d Compare August 11, 2026 01:58

JingsongLi reviewed Aug 11, 2026

View reviewed changes

Stephen0421 force-pushed the pypaimon-blob-pr1-foundation branch from 0c0865d to a7975e5 Compare August 11, 2026 11:44

JingsongLi reviewed Aug 13, 2026

View reviewed changes

Stephen0421 force-pushed the pypaimon-blob-pr1-foundation branch from a7975e5 to dc471d0 Compare August 13, 2026 06:37

Stephen0421 force-pushed the pypaimon-blob-pr1-foundation branch from dc471d0 to 414804e Compare August 13, 2026 07:50

Stephen0421 requested a review from JingsongLi August 14, 2026 07:29

Uh oh!

Conversation

Stephen0421 commented Aug 10, 2026

Summary

Behavior changes (intentional)

Test plan

Follow-ups

Uh oh!

JingsongLi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JingsongLi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JingsongLi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants