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