You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "Stephen0421 (via GitHub)" <gi...@apache.org> on 2026/08/10 10:13:00 UTC
[PR] [python] BLOB descriptor parsing and read-path compatibility. [paimon]
Stephen0421 opened a new pull request, #9148:
URL: https://github.com/apache/paimon/pull/9148
## 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
- [x] `BlobTest`
- [x] `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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [python] BLOB descriptor parsing and read-path compatibility. [paimon]
Posted by "Stephen0421 (via GitHub)" <gi...@apache.org>.
Stephen0421 commented on code in PR #9148:
URL: https://github.com/apache/paimon/pull/9148#discussion_r3790818853
##########
paimon-python/pypaimon/common/options/core_options.py:
##########
@@ -1213,7 +1222,14 @@ def variant_shredding_schema(self) -> Optional[str]:
return val
def blob_descriptor_fields(self, default=None):
- value = self.options.get(CoreOptions.BLOB_DESCRIPTOR_FIELD, default)
+ value = self.options.get(CoreOptions.BLOB_DESCRIPTOR_FIELD, None)
Review Comment:
Agreed. Python master ignored that key and wrote dedicated `.blob` payloads, so treating it as a global `blob-descriptor-field` fallback is unsafe for rolling upgrades. `blob_descriptor_fields()` now only honors the canonical `blob-descriptor-field`. Column directives still migrate the legacy key onto the canonical option. Added master-layout write/read coverage with the legacy key only (`test_legacy_stored_descriptor_fields_keeps_dedicated_blob_layout`, `test_blob_descriptor_fields_ignores_legacy_stored_key`).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [python] BLOB descriptor parsing and read-path compatibility. [paimon]
Posted by "Stephen0421 (via GitHub)" <gi...@apache.org>.
Stephen0421 commented on code in PR #9148:
URL: https://github.com/apache/paimon/pull/9148#discussion_r3819326763
##########
paimon-python/pypaimon/utils/blob_view_lookup.py:
##########
@@ -139,6 +137,22 @@ def resolve_uri_reader(self, view_struct: BlobViewStruct) -> UriReader:
)
return uri_reader
+ def resolve_uri_reader_for_uri(self, uri: str) -> Optional[UriReader]:
+ """UriReader bound to the upstream table that produced this descriptor."""
+ return self._uri_reader_by_uri.get(uri)
+
+ def _store_chunk_results(self, descriptors, null_values):
+ self._descriptor_cache.update(descriptors)
+ self._null_value_cache.update(null_values)
+ for view_struct, descriptor in descriptors.items():
+ factory = self._uri_reader_factory_cache.get(
+ view_struct.identifier.get_full_name())
+ if factory is not None:
+ # from_file_io: HTTP(S) stays on HttpUriReader; other URIs
+ # reuse the upstream table FileIO (REST table token).
+ self._uri_reader_by_uri[descriptor.uri] = factory.create(
Review Comment:
Thanks for catching this. Fixed the remaining reader-ownership issue by preserving the originating `BlobViewStruct` identity instead of collapsing readers into a URI-only mapping.
Specifically:
- `BlobViewLookup` now resolves each view through its source table’s `UriReaderFactory`.
- The row path keeps the original `BlobViewStruct` until `get_blob()`, so the source-table association is not lost.
- Inline materialization carries the resolved `BlobRef`, and concurrent reads honor each ref’s own reader, including HTTP(S).
- Added a regression with two upstream table identifiers using different token-aware FileIOs but the same descriptor URI. Each row resolves through its own source FileIO, and the target table FileIO is not used.
- Added coverage for HTTP serial/parallel materialization and related reader-metadata and compatibility cases.
Verification:
- BLOB/URI/REST/vector tests: `191 passed, 2 skipped`
- Full `blob_table_test.py`: `90 passed, 1 skipped`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [python] BLOB descriptor parsing and read-path compatibility. [paimon]
Posted by "Stephen0421 (via GitHub)" <gi...@apache.org>.
Stephen0421 commented on code in PR #9148:
URL: https://github.com/apache/paimon/pull/9148#discussion_r3773752586
##########
paimon-python/pypaimon/read/split_read.py:
##########
@@ -1003,6 +1047,23 @@ def _build_merge_function(self):
value_field_names=[f.name for f in self.value_fields],
)
+ def _create_blob_view_prescan_reader(self, field_names: set):
+ 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()
Review Comment:
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.
##########
paimon-python/pypaimon/table/row/offset_row.py:
##########
@@ -55,12 +62,57 @@ def get_field(self, pos: int):
raise IndexError(f"Position {pos} is out of bounds for row arity {self.arity}")
return self.row_tuple[self.offset + pos]
- def get_blob(self, pos: int):
+ @staticmethod
+ def _normalize_blob_bytes(value):
+ if value is None:
+ return None
+ if hasattr(value, 'as_py'):
+ value = value.as_py()
+ if isinstance(value, str):
+ value = value.encode('utf-8')
+ if isinstance(value, bytearray):
+ value = bytes(value)
+ return value
+
+ def _resolve_blob_view_struct(self, view_struct):
from pypaimon.table.row.blob import Blob
+ if self._blob_view_lookup is not None:
+ if self._blob_view_lookup.resolve_to_null(view_struct):
+ return None
+ descriptor = self._blob_view_lookup.resolve_descriptor(view_struct)
+ uri_reader = self._blob_view_lookup.resolve_uri_reader(view_struct)
+ return Blob.from_descriptor(uri_reader, descriptor)
+ return Blob.from_view(view_struct)
+
+ def _blob_from_descriptor_field_bytes(self, raw: bytes):
+ from pypaimon.table.row.blob import Blob, BlobDescriptor
+
+ if BlobDescriptor.is_blob_descriptor(raw):
+ return Blob.from_descriptor_bytes(raw, self._file_io)
+ if BlobDescriptor.parse_if_serialized(raw) is not None:
+ return Blob.from_descriptor_bytes(raw, self._file_io)
+ try:
+ # Accept v1/v2 descriptors with trailing padding (Java deserialize).
+ return Blob.from_descriptor_bytes(raw, self._file_io)
+ except ValueError:
Review Comment:
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [python] BLOB descriptor parsing and read-path compatibility. [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #9148:
URL: https://github.com/apache/paimon/pull/9148#issuecomment-5312231014
When `blob-as-descriptor=true` is enabled for a cross-table BLOB view, the source table's `FileIO/token` is lost; subsequent calls to `get_blob().to_data()` attempt to read the source table's `.blob` using the target table's token, which fails under REST Catalog table-level permissions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [python] BLOB descriptor parsing and read-path compatibility. [paimon]
Posted by "Stephen0421 (via GitHub)" <gi...@apache.org>.
Stephen0421 commented on code in PR #9148:
URL: https://github.com/apache/paimon/pull/9148#discussion_r3790819093
##########
paimon-python/pypaimon/read/reader/blob_descriptor_convert_reader.py:
##########
@@ -239,5 +245,15 @@ def _normalize_blob_to_bytes(value):
value = bytes(value)
return value
+ @staticmethod
+ def _descriptor_field_to_blob(value, file_io):
+ if value is None:
+ return None
+ return Blob.from_descriptor_bytes(
+ value,
+ file_io=file_io,
+ uri_reader_factory=getattr(file_io, 'uri_reader_factory', None),
Review Comment:
Agreed. Descriptor materialization now uses `UriReaderFactory.from_file_io` (HTTP(S) → `HttpUriReader`, other URIs → the supplied table FileIO). `OffsetRow.get_blob()` and `BlobInlineConvertReader` no longer consult `file_io.uri_reader_factory`. Added a token-aware FileIO regression that fails if the catalog factory is used.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [python] BLOB descriptor parsing and read-path compatibility. [paimon]
Posted by "Stephen0421 (via GitHub)" <gi...@apache.org>.
Stephen0421 commented on code in PR #9148:
URL: https://github.com/apache/paimon/pull/9148#discussion_r3754761788
##########
paimon-python/pypaimon/write/blob_format_writer.py:
##########
@@ -290,17 +290,44 @@ def _write_blob_data(self, blob_value: Blob, crc32: int):
data = blob_value.to_data()
crc32 = self._write_with_crc(data, crc32)
else:
+ expected_length = self._expected_blob_length(blob_value)
Review Comment:
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.
##########
paimon-python/pypaimon/common/options/core_options.py:
##########
@@ -1213,7 +1222,18 @@ def variant_shredding_schema(self) -> Optional[str]:
return val
def blob_descriptor_fields(self, default=None):
- value = self.options.get(CoreOptions.BLOB_DESCRIPTOR_FIELD, default)
+ value = self.options.get(CoreOptions.BLOB_DESCRIPTOR_FIELD, None)
+ if isinstance(value, str):
+ value = value.strip()
+ if not value:
Review Comment:
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.
##########
paimon-python/pypaimon/table/row/blob.py:
##########
@@ -118,6 +118,53 @@ def deserialize(cls, data: bytes) -> 'BlobDescriptor':
descriptor._version = version
return descriptor
+ @classmethod
+ def _try_parse_serialized(cls, data: bytes) -> Optional['BlobDescriptor']:
+ if not isinstance(data, (bytes, bytearray)):
+ return None
+ raw = bytes(data)
+ if len(raw) < 21:
+ return None
+ try:
+ offset = 0
+ version = raw[offset]
+ offset += 1
+ if version < 1 or version > cls.CURRENT_VERSION:
+ return None
+ if version > 1:
+ if offset + 8 > len(raw):
+ return None
+ magic = struct.unpack('<Q', raw[offset:offset + 8])[0]
+ if magic != cls.MAGIC:
+ return None
+ offset += 8
+ if offset + 4 > len(raw):
+ return None
+ uri_length = struct.unpack('<I', raw[offset:offset + 4])[0]
+ total = offset + 4 + uri_length + 16
+ if total != len(raw):
Review Comment:
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [python] BLOB descriptor parsing and read-path compatibility. [paimon]
Posted by "Stephen0421 (via GitHub)" <gi...@apache.org>.
Stephen0421 commented on PR #9148:
URL: https://github.com/apache/paimon/pull/9148#issuecomment-5503863486
> Could you break this down into separate PRs for each feature? There are simply too many changes right now, making it very difficult to review.
Thanks, I will do the separation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [python] BLOB descriptor parsing and read-path compatibility. [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #9148:
URL: https://github.com/apache/paimon/pull/9148#issuecomment-5436086965
Could you break this down into separate PRs for each feature? There are simply too many changes right now, making it very difficult to review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [python] BLOB descriptor parsing and read-path compatibility. [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #9148:
URL: https://github.com/apache/paimon/pull/9148#discussion_r3756882005
##########
paimon-python/pypaimon/read/reader/field_indices.py:
##########
@@ -29,6 +29,23 @@ def blob_field_indices(fields: List[DataField]) -> Set[int]:
}
+def descriptor_field_indices(
+ fields: List[DataField], descriptor_field_names: Iterable[str]) -> Set[int]:
+ names = set(descriptor_field_names)
+ if not names:
+ return set()
+ return {i for i, f in enumerate(fields) if f.name in names}
+
+
+def descriptor_field_indices_for_table(table, fields: List[DataField]) -> Set[int]:
+ from pypaimon.common.options.core_options import CoreOptions
+
+ if not CoreOptions.blob_as_descriptor(table.options):
+ return set()
+ return descriptor_field_indices(
Review Comment:
[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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [python] BLOB descriptor parsing and read-path compatibility. [paimon]
Posted by "Stephen0421 (via GitHub)" <gi...@apache.org>.
Stephen0421 commented on code in PR #9148:
URL: https://github.com/apache/paimon/pull/9148#discussion_r3773750771
##########
paimon-python/pypaimon/read/reader/blob_view_read_support.py:
##########
@@ -0,0 +1,53 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Helpers for eager blob-view/descriptor inline conversion on read."""
+
+from typing import List
+
+from pypaimon.common.options.core_options import CoreOptions
+from pypaimon.read.reader.iface.record_reader import RecordReader
+from pypaimon.schema.data_types import DataField, PyarrowFieldParser
+
+
+def needs_blob_inline_convert(table) -> bool:
+ return (
+ (CoreOptions.blob_view_fields(table.options)
+ and CoreOptions.blob_view_resolve_enabled(table.options))
+ or (not CoreOptions.blob_as_descriptor(table.options)
+ and CoreOptions.blob_descriptor_fields(table.options))
+ )
+
+
+def wrap_record_reader_with_blob_inline_convert(
+ reader: RecordReader,
+ split_read,
+ read_fields: List[DataField],
+) -> RecordReader:
+ from pypaimon.read.reader.auth_masking_reader import (
+ BatchToRecordReaderAdapter, RecordReaderToBatchAdapter)
+ from pypaimon.read.reader.blob_descriptor_convert_reader import BlobInlineConvertReader
+
+ schema = PyarrowFieldParser.from_paimon_schema(read_fields)
+ batch_reader = RecordReaderToBatchAdapter(reader, schema)
+ batch_reader = BlobInlineConvertReader(
+ batch_reader,
+ split_read.table,
+ prescan_reader_factory=lambda names: split_read._create_blob_view_prescan_reader(names),
+ blob_parallelism=split_read._blob_parallelism,
+ )
+ return BatchToRecordReaderAdapter(batch_reader)
Review Comment:
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()`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [python] BLOB descriptor parsing and read-path compatibility. [paimon]
Posted by "Stephen0421 (via GitHub)" <gi...@apache.org>.
Stephen0421 commented on code in PR #9148:
URL: https://github.com/apache/paimon/pull/9148#discussion_r3773754005
##########
paimon-python/pypaimon/write/blob_format_writer.py:
##########
@@ -290,17 +290,38 @@ def _write_blob_data(self, blob_value: Blob, crc32: int):
data = blob_value.to_data()
crc32 = self._write_with_crc(data, crc32)
else:
+ expected_length = None
+ if type(blob_value) is BlobRef:
+ descriptor_length = blob_value.to_descriptor().length
+ if descriptor_length >= 0:
+ expected_length = descriptor_length
stream = blob_value.new_input_stream()
try:
- chunk = stream.read(self.copy_buffer_size)
- while chunk:
- crc32 = self._write_with_crc(chunk, crc32)
+ if expected_length is not None:
+ crc32 = self._copy_exactly(stream, expected_length, crc32)
+ else:
chunk = stream.read(self.copy_buffer_size)
+ while chunk:
+ crc32 = self._write_with_crc(chunk, crc32)
+ chunk = stream.read(self.copy_buffer_size)
finally:
stream.close()
Review Comment:
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [python] BLOB descriptor parsing and read-path compatibility. [paimon]
Posted by "Stephen0421 (via GitHub)" <gi...@apache.org>.
Stephen0421 commented on code in PR #9148:
URL: https://github.com/apache/paimon/pull/9148#discussion_r3790819538
##########
paimon-python/pypaimon/read/reader/blob_view_read_support.py:
##########
@@ -0,0 +1,65 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Helpers for eager blob-view/descriptor inline conversion on read."""
+
+from typing import List
+
+from pypaimon.common.options.core_options import CoreOptions
+from pypaimon.read.reader.iface.record_reader import RecordReader
+from pypaimon.schema.data_types import DataField, PyarrowFieldParser
+
+
+def needs_blob_inline_convert(table) -> bool:
+ view_fields = CoreOptions.blob_view_fields(table.options)
+ descriptor_fields = CoreOptions.blob_descriptor_fields(table.options)
+ if descriptor_fields:
+ # Materialize when blob-as-descriptor=false; otherwise still wrap so
+ # merge to_iterator()+get_blob() receives descriptor field metadata.
+ return True
+ if not view_fields:
+ return False
+ if CoreOptions.blob_as_descriptor(table.options):
+ return True
+ return CoreOptions.blob_view_resolve_enabled(table.options)
+
+
+def wrap_record_reader_with_blob_inline_convert(
+ reader: RecordReader,
+ split_read,
+ read_fields: List[DataField],
+) -> RecordReader:
+ from pypaimon.read.reader.auth_masking_reader import (
+ BatchToRecordReaderAdapter, RecordReaderToBatchAdapter)
+ from pypaimon.read.reader.blob_descriptor_convert_reader import BlobInlineConvertReader
+ from pypaimon.read.reader.field_indices import (
+ blob_field_indices, descriptor_field_indices_for_table, vector_field_indices)
+
+ schema = PyarrowFieldParser.from_paimon_schema(read_fields)
+ batch_reader = RecordReaderToBatchAdapter(reader, schema)
Review Comment:
Fixed. The merge wrap round-trip now sets `include_row_kind=True` so `BatchToRecordReaderAdapter` restores the original kind instead of defaulting to `-U`. Added coverage for `+I` / `-U` / `+U` / `-D` (`test_wrap_record_reader_preserves_all_row_kinds`).
##########
paimon-python/pypaimon/read/split_read.py:
##########
@@ -1017,16 +1083,25 @@ def create_reader(self) -> RecordReader:
reader = FilterRecordReader(kv_unwrap_reader, self.predicate_for_reader)
else:
reader = kv_unwrap_reader
+ value_fields = self.read_fields[-self.value_arity:]
+ if self._needs_blob_inline_convert() and not self._blob_view_prescan:
+ reader = wrap_record_reader_with_blob_inline_convert(
+ reader, self, value_fields)
Review Comment:
Fixed. Merge now applies LIMIT before inline convert when the nested-leaf predicate is not re-applied after projection, so descriptors/views are not materialized past LIMIT. Prescan LIMIT is skipped when a predicate or auth filter is present (same as DataEvolution), because prescan only projects view columns and the filtered first-N would otherwise miss the lookup. Added LIMIT-only materialize coverage, DataEvolution predicate+LIMIT, and a RawFileSplitRead predicate+LIMIT regression (`id == 9` with `LIMIT 1`).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [python] BLOB descriptor parsing and read-path compatibility. [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #9148:
URL: https://github.com/apache/paimon/pull/9148#discussion_r3788778789
##########
paimon-python/pypaimon/common/options/core_options.py:
##########
@@ -1213,7 +1222,14 @@ def variant_shredding_schema(self) -> Optional[str]:
return val
def blob_descriptor_fields(self, default=None):
- value = self.options.get(CoreOptions.BLOB_DESCRIPTOR_FIELD, default)
+ value = self.options.get(CoreOptions.BLOB_DESCRIPTOR_FIELD, None)
Review Comment:
**[P1] Preserve mixed-version compatibility for legacy-only tables.** This newly interprets `blob.stored-descriptor-fields` as inline `blob-descriptor-field` for every historical file, while current master ignored that key and wrote the same BLOB column as a dedicated `.blob` payload. I reproduced current-master write -> this-head read failing with `ValueError: Expected BlobDescriptor bytes, got raw bytes`; the reverse direction makes the old reader return descriptor wire bytes instead of the payload, and an old application that still submits raw BLOB bytes is rejected by the new writer. Please distinguish the physical layout per file (for example from `write_cols` / file type), or introduce an explicit migration/compatibility path, and add base-write/head-read plus head-write/base-read tests. A global option fallback is not safe during a rolling upgrade.
##########
paimon-python/pypaimon/read/reader/blob_descriptor_convert_reader.py:
##########
@@ -239,5 +245,15 @@ def _normalize_blob_to_bytes(value):
value = bytes(value)
return value
+ @staticmethod
+ def _descriptor_field_to_blob(value, file_io):
+ if value is None:
+ return None
+ return Blob.from_descriptor_bytes(
+ value,
+ file_io=file_io,
+ uri_reader_factory=getattr(file_io, 'uri_reader_factory', None),
Review Comment:
**[P1] Keep table-scoped credentials for non-HTTP descriptors.** `RESTTokenFileIO` refreshes and merges the server-issued table data token only through `file_io()` / `new_input_stream()`, but its `uri_reader_factory` is built from the raw catalog options. Selecting that factory here therefore rebuilds an unscoped FileIO and makes default descriptor materialization fail with OSS/S3 403 when access exists only through the table token; `OffsetRow.get_blob()` now has the same regression. I reproduced the factory path failing while the supplied table FileIO reads the same descriptor successfully. Please mirror Java's `UriReaderFactory.fromFileIO`: reuse the supplied, token-aware FileIO for non-HTTP URIs and keep the HTTP reader for HTTP(S), with a REST table-token regression test.
##########
paimon-python/pypaimon/read/split_read.py:
##########
@@ -1017,16 +1083,25 @@ def create_reader(self) -> RecordReader:
reader = FilterRecordReader(kv_unwrap_reader, self.predicate_for_reader)
else:
reader = kv_unwrap_reader
+ value_fields = self.read_fields[-self.value_arity:]
+ if self._needs_blob_inline_convert() and not self._blob_view_prescan:
+ reader = wrap_record_reader_with_blob_inline_convert(
+ reader, self, value_fields)
Review Comment:
**[P1] Make the BlobView prescan and main Merge reader consume the same limited row set.** The prescan reader is constructed with `limit=self.limit` and preloads only N views, but this main adapter can pull up to its 65,536-row chunk and resolve all of them before the outer `LimitedRecordReader` at the end of `create_reader()` truncates to N. With two view rows and `LIMIT 1`, resolving row 2 deterministically fails because it was not preloaded; descriptor fields can likewise materialize an entire chunk of large BLOBs before `LIMIT 1`. Please apply filtering/LIMIT before inline conversion when safe, or stop limiting the prescan and otherwise guarantee identical selection/backpressure on both paths. Add Merge BlobView + LIMIT and predicate + LIMIT regressions.
##########
paimon-python/pypaimon/read/reader/blob_view_read_support.py:
##########
@@ -0,0 +1,65 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Helpers for eager blob-view/descriptor inline conversion on read."""
+
+from typing import List
+
+from pypaimon.common.options.core_options import CoreOptions
+from pypaimon.read.reader.iface.record_reader import RecordReader
+from pypaimon.schema.data_types import DataField, PyarrowFieldParser
+
+
+def needs_blob_inline_convert(table) -> bool:
+ view_fields = CoreOptions.blob_view_fields(table.options)
+ descriptor_fields = CoreOptions.blob_descriptor_fields(table.options)
+ if descriptor_fields:
+ # Materialize when blob-as-descriptor=false; otherwise still wrap so
+ # merge to_iterator()+get_blob() receives descriptor field metadata.
+ return True
+ if not view_fields:
+ return False
+ if CoreOptions.blob_as_descriptor(table.options):
+ return True
+ return CoreOptions.blob_view_resolve_enabled(table.options)
+
+
+def wrap_record_reader_with_blob_inline_convert(
+ reader: RecordReader,
+ split_read,
+ read_fields: List[DataField],
+) -> RecordReader:
+ from pypaimon.read.reader.auth_masking_reader import (
+ BatchToRecordReaderAdapter, RecordReaderToBatchAdapter)
+ from pypaimon.read.reader.blob_descriptor_convert_reader import BlobInlineConvertReader
+ from pypaimon.read.reader.field_indices import (
+ blob_field_indices, descriptor_field_indices_for_table, vector_field_indices)
+
+ schema = PyarrowFieldParser.from_paimon_schema(read_fields)
+ batch_reader = RecordReaderToBatchAdapter(reader, schema)
Review Comment:
**[P1] Preserve RowKind across this row -> Arrow -> row bridge.** `RecordReaderToBatchAdapter` defaults `include_row_kind` to false, so the batch has no `_row_kind`; `BatchToRecordReaderAdapter` then creates an `OffsetRow` with the default byte `1` (`-U`). A target-head reproducer changes an input `+U` into `-U`, and the Merge path enters this helper whenever the table has a descriptor/view field even if the current projection excludes it. This corrupts CDC / `include_row_kind=true` semantics. Please carry and restore RowKind unconditionally for this internal round-trip (for example, pass `include_row_kind=True`) and add a Merge regression covering all four row kinds.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [python] BLOB descriptor parsing and read-path compatibility. [paimon]
Posted by "Stephen0421 (via GitHub)" <gi...@apache.org>.
Stephen0421 closed pull request #9148: [python] BLOB descriptor parsing and read-path compatibility.
URL: https://github.com/apache/paimon/pull/9148
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [python] BLOB descriptor parsing and read-path compatibility. [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #9148:
URL: https://github.com/apache/paimon/pull/9148#discussion_r3818249072
##########
paimon-python/pypaimon/utils/blob_view_lookup.py:
##########
@@ -139,6 +137,22 @@ def resolve_uri_reader(self, view_struct: BlobViewStruct) -> UriReader:
)
return uri_reader
+ def resolve_uri_reader_for_uri(self, uri: str) -> Optional[UriReader]:
+ """UriReader bound to the upstream table that produced this descriptor."""
+ return self._uri_reader_by_uri.get(uri)
+
+ def _store_chunk_results(self, descriptors, null_values):
+ self._descriptor_cache.update(descriptors)
+ self._null_value_cache.update(null_values)
+ for view_struct, descriptor in descriptors.items():
+ factory = self._uri_reader_factory_cache.get(
+ view_struct.identifier.get_full_name())
+ if factory is not None:
+ # from_file_io: HTTP(S) stays on HttpUriReader; other URIs
+ # reuse the upstream table FileIO (REST table token).
+ self._uri_reader_by_uri[descriptor.uri] = factory.create(
Review Comment:
[P1] Keep reader ownership per originating BlobView, not only per URI. Two upstream tables can legally produce the same descriptor URI while carrying different REST table-scoped credentials. `_store_chunk_results` then overwrites the first reader with whichever table finishes last (nondeterministically in the parallel path), and `OffsetRow._blob_from_descriptor_field_bytes` routes every matching descriptor through that one token. I reproduced this with two token-aware FileIOs and the same `s3://shared/blob` descriptor: after storing A then B, both rows read B. Under real table-level permissions, one source therefore gets a 403. Please preserve the source-table/row association through Stage 1 instead of collapsing it to a URI-only map, and add a two-upstream-table/same-URI regression.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [python] BLOB descriptor parsing and read-path compatibility. [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #9148:
URL: https://github.com/apache/paimon/pull/9148#discussion_r3749922100
##########
paimon-python/pypaimon/common/options/core_options.py:
##########
@@ -1213,7 +1222,18 @@ def variant_shredding_schema(self) -> Optional[str]:
return val
def blob_descriptor_fields(self, default=None):
- value = self.options.get(CoreOptions.BLOB_DESCRIPTOR_FIELD, default)
+ value = self.options.get(CoreOptions.BLOB_DESCRIPTOR_FIELD, None)
+ if isinstance(value, str):
+ value = value.strip()
+ if not value:
Review Comment:
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.
##########
paimon-python/pypaimon/table/row/blob.py:
##########
@@ -118,6 +118,53 @@ def deserialize(cls, data: bytes) -> 'BlobDescriptor':
descriptor._version = version
return descriptor
+ @classmethod
+ def _try_parse_serialized(cls, data: bytes) -> Optional['BlobDescriptor']:
+ if not isinstance(data, (bytes, bytearray)):
+ return None
+ raw = bytes(data)
+ if len(raw) < 21:
+ return None
+ try:
+ offset = 0
+ version = raw[offset]
+ offset += 1
+ if version < 1 or version > cls.CURRENT_VERSION:
+ return None
+ if version > 1:
+ if offset + 8 > len(raw):
+ return None
+ magic = struct.unpack('<Q', raw[offset:offset + 8])[0]
+ if magic != cls.MAGIC:
+ return None
+ offset += 8
+ if offset + 4 > len(raw):
+ return None
+ uri_length = struct.unpack('<I', raw[offset:offset + 4])[0]
+ total = offset + 4 + uri_length + 16
+ if total != len(raw):
Review Comment:
`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.
##########
paimon-python/pypaimon/write/blob_format_writer.py:
##########
@@ -290,17 +290,44 @@ def _write_blob_data(self, blob_value: Blob, crc32: int):
data = blob_value.to_data()
crc32 = self._write_with_crc(data, crc32)
else:
+ expected_length = self._expected_blob_length(blob_value)
Review Comment:
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [python] BLOB descriptor parsing and read-path compatibility. [paimon]
Posted by "Stephen0421 (via GitHub)" <gi...@apache.org>.
Stephen0421 commented on code in PR #9148:
URL: https://github.com/apache/paimon/pull/9148#discussion_r3757652962
##########
paimon-python/pypaimon/read/reader/field_indices.py:
##########
@@ -29,6 +29,23 @@ def blob_field_indices(fields: List[DataField]) -> Set[int]:
}
+def descriptor_field_indices(
+ fields: List[DataField], descriptor_field_names: Iterable[str]) -> Set[int]:
+ names = set(descriptor_field_names)
+ if not names:
+ return set()
+ return {i for i, f in enumerate(fields) if f.name in names}
+
+
+def descriptor_field_indices_for_table(table, fields: List[DataField]) -> Set[int]:
+ from pypaimon.common.options.core_options import CoreOptions
+
+ if not CoreOptions.blob_as_descriptor(table.options):
+ return set()
+ return descriptor_field_indices(
Review Comment:
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [python] BLOB descriptor parsing and read-path compatibility. [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #9148:
URL: https://github.com/apache/paimon/pull/9148#discussion_r3771892660
##########
paimon-python/pypaimon/read/reader/blob_view_read_support.py:
##########
@@ -0,0 +1,53 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Helpers for eager blob-view/descriptor inline conversion on read."""
+
+from typing import List
+
+from pypaimon.common.options.core_options import CoreOptions
+from pypaimon.read.reader.iface.record_reader import RecordReader
+from pypaimon.schema.data_types import DataField, PyarrowFieldParser
+
+
+def needs_blob_inline_convert(table) -> bool:
+ return (
+ (CoreOptions.blob_view_fields(table.options)
+ and CoreOptions.blob_view_resolve_enabled(table.options))
+ or (not CoreOptions.blob_as_descriptor(table.options)
+ and CoreOptions.blob_descriptor_fields(table.options))
+ )
+
+
+def wrap_record_reader_with_blob_inline_convert(
+ reader: RecordReader,
+ split_read,
+ read_fields: List[DataField],
+) -> RecordReader:
+ from pypaimon.read.reader.auth_masking_reader import (
+ BatchToRecordReaderAdapter, RecordReaderToBatchAdapter)
+ from pypaimon.read.reader.blob_descriptor_convert_reader import BlobInlineConvertReader
+
+ schema = PyarrowFieldParser.from_paimon_schema(read_fields)
+ batch_reader = RecordReaderToBatchAdapter(reader, schema)
+ batch_reader = BlobInlineConvertReader(
+ batch_reader,
+ split_read.table,
+ prescan_reader_factory=lambda names: split_read._create_blob_view_prescan_reader(names),
+ blob_parallelism=split_read._blob_parallelism,
+ )
+ return BatchToRecordReaderAdapter(batch_reader)
Review Comment:
[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.
##########
paimon-python/pypaimon/read/split_read.py:
##########
@@ -1003,6 +1047,23 @@ def _build_merge_function(self):
value_field_names=[f.name for f in self.value_fields],
)
+ def _create_blob_view_prescan_reader(self, field_names: set):
+ 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()
Review Comment:
[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.
##########
paimon-python/pypaimon/write/blob_format_writer.py:
##########
@@ -290,17 +290,38 @@ def _write_blob_data(self, blob_value: Blob, crc32: int):
data = blob_value.to_data()
crc32 = self._write_with_crc(data, crc32)
else:
+ expected_length = None
+ if type(blob_value) is BlobRef:
+ descriptor_length = blob_value.to_descriptor().length
+ if descriptor_length >= 0:
+ expected_length = descriptor_length
stream = blob_value.new_input_stream()
try:
- chunk = stream.read(self.copy_buffer_size)
- while chunk:
- crc32 = self._write_with_crc(chunk, crc32)
+ if expected_length is not None:
+ crc32 = self._copy_exactly(stream, expected_length, crc32)
+ else:
chunk = stream.read(self.copy_buffer_size)
+ while chunk:
+ crc32 = self._write_with_crc(chunk, crc32)
+ chunk = stream.read(self.copy_buffer_size)
finally:
stream.close()
Review Comment:
[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.
##########
paimon-python/pypaimon/table/row/offset_row.py:
##########
@@ -55,12 +62,57 @@ def get_field(self, pos: int):
raise IndexError(f"Position {pos} is out of bounds for row arity {self.arity}")
return self.row_tuple[self.offset + pos]
- def get_blob(self, pos: int):
+ @staticmethod
+ def _normalize_blob_bytes(value):
+ if value is None:
+ return None
+ if hasattr(value, 'as_py'):
+ value = value.as_py()
+ if isinstance(value, str):
+ value = value.encode('utf-8')
+ if isinstance(value, bytearray):
+ value = bytes(value)
+ return value
+
+ def _resolve_blob_view_struct(self, view_struct):
from pypaimon.table.row.blob import Blob
+ if self._blob_view_lookup is not None:
+ if self._blob_view_lookup.resolve_to_null(view_struct):
+ return None
+ descriptor = self._blob_view_lookup.resolve_descriptor(view_struct)
+ uri_reader = self._blob_view_lookup.resolve_uri_reader(view_struct)
+ return Blob.from_descriptor(uri_reader, descriptor)
+ return Blob.from_view(view_struct)
+
+ def _blob_from_descriptor_field_bytes(self, raw: bytes):
+ from pypaimon.table.row.blob import Blob, BlobDescriptor
+
+ if BlobDescriptor.is_blob_descriptor(raw):
+ return Blob.from_descriptor_bytes(raw, self._file_io)
+ if BlobDescriptor.parse_if_serialized(raw) is not None:
+ return Blob.from_descriptor_bytes(raw, self._file_io)
+ try:
+ # Accept v1/v2 descriptors with trailing padding (Java deserialize).
+ return Blob.from_descriptor_bytes(raw, self._file_io)
+ except ValueError:
Review Comment:
[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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [python] BLOB descriptor parsing and read-path compatibility. [paimon]
Posted by "Stephen0421 (via GitHub)" <gi...@apache.org>.
Stephen0421 commented on PR #9148:
URL: https://github.com/apache/paimon/pull/9148#issuecomment-5314779123
> When `blob-as-descriptor=true` is enabled for a cross-table BLOB view, the source table's `FileIO/token` is lost; subsequent calls to `get_blob().to_data()` attempt to read the source table's `.blob` using the target table's token, which fails under REST Catalog table-level permissions.
Agreed. When `blob-as-descriptor=true`, Stage 1 rewrote BlobView structs to source descriptor bytes and dropped the upstream FileIO, so `get_blob().to_data()` used the target table token.
Prescan now indexes each descriptor URI through `UriReaderFactory.from_file_io` on the upstream table (HTTP(S) stays on `HttpUriReader`; other URIs reuse the source FileIO). `OffsetRow.get_blob()` prefers that reader. The convert reader exposes `blob_view_lookup` after prescan, and the row↔batch / filter / limit / outer-projection wrappers refresh it after the first read (lookup is filled during prescan, after wrap `__init__`).
Added a cross-table regression that fails if the target FileIO opens the source `.blob`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org