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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
