[python] BLOB descriptor parsing and read-path compatibility. - #9148
[python] BLOB descriptor parsing and read-path compatibility.#9148Stephen0421 wants to merge 1 commit into
Conversation
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.
JingsongLi
left a comment
JingsongLi
left a comment
There was a problem hiding this comment.
Inline comments focus on behavioral regressions and consistency with the Java implementation.
| data = blob_value.to_data() | ||
| crc32 = self._write_with_crc(data, crc32) | ||
| else: | ||
| expected_length = self._expected_blob_length(blob_value) |
There was a problem hiding this 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.
| value = self.options.get(CoreOptions.BLOB_DESCRIPTOR_FIELD, None) | ||
| if isinstance(value, str): | ||
| value = value.strip() | ||
| if not value: |
There was a problem hiding this 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.
| return None | ||
| uri_length = struct.unpack('<I', raw[offset:offset + 4])[0] | ||
| total = offset + 4 + uri_length + 16 | ||
| if total != len(raw): |
There was a problem hiding this 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.
2 participants
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.
blob.py):from_bytes(v2 magic heuristic) vsfrom_descriptor_bytes(v1 strict + v2 deserialize with optional trailing padding)BlobInlineConvertReaderusesfrom_descriptor_bytesfor descriptor fieldsdescriptor_field_indiceswhenblob-as-descriptor=true→OffsetRow.get_blob()usesfrom_descriptor_bytesblob.stored-descriptor-fields; blankblob-descriptor-fieldtreated as unsetblob_format_writerrejects truncated copies (EOFError) when descriptor length is knownUriReaderFactoryowned FileIO tracking;clear_cache()without LRU double-close; FileIOclose()wiringBehavior changes (intentional)
ValueError(was silentBlobDatapassthrough)blob-descriptor-field=""+ legacy set → now falls back toblob.stored-descriptor-fieldsfrom_byteson arbitrary inline payload → unchanged (BlobData; v2-only heuristic)Test plan
BlobTestUriReaderFactoryTestFollow-ups
Related: #9099