JingsongLi commented on code in PR #9428:
URL: https://github.com/apache/paimon/pull/9428#discussion_r3878290764
##########
paimon-python/pypaimon/multimodal/blob_read.py:
##########
@@ -19,43 +19,82 @@
def fetch_blob_bodies(file_io, data, blob_cols, parallelism):
- """Fetch BLOB payload bytes for descriptor/inline/null cells.
+ """Fetch scalar and MAP BLOB payload bytes.
``data`` is a ``dict`` mapping each BLOB column name to row-aligned cells.
- Each cell may be serialized ``BlobDescriptor`` bytes, inline payload bytes,
- or ``None``. Returned values preserve row order and are grouped per column.
+ A cell may be serialized ``BlobDescriptor`` bytes, inline payload bytes,
+ ``None``, or a MAP represented by key-value pairs. Returned values preserve
+ row and MAP entry order and are grouped per column.
"""
from pypaimon.table.row.blob import BlobDescriptor, BlobViewStruct
ranges = []
inline = {}
- index = 0
+ targets = []
+ bodies = {col: [] for col in blob_cols}
+
+ def queue_blob_fetch(value, target):
+ index = len(ranges)
+ if value is None:
+ ranges.append(None)
+ else:
+ raw = bytes(value)
+ if BlobViewStruct.is_blob_view_struct(raw):
+ raise ValueError(
+ "read_blobs does not support unresolved blob-view columns;
"
+ "read such a column on its own, or enable blob-view
resolution.")
+ if BlobDescriptor.is_blob_descriptor(raw):
+ descriptor = BlobDescriptor.deserialize(raw)
+ ranges.append(
+ (descriptor.uri, descriptor.offset, descriptor.length)
+ )
+ else:
+ ranges.append(None)
+ inline[index] = raw
+ targets.append((target, index))
Review Comment:
[P2] Avoid per-cell target bookkeeping for scalar BLOB reads
This sends every non-null scalar BLOB cell through the generic target
mapping, retaining two target tuples plus the bodies and ranges lists until all
reads complete. In an isolated 300k-descriptor run, peak allocations increased
from 41.0 MiB on the base to 94.0 MiB on this head; for inline cells they
increased from 26.2 MiB to 70.1 MiB, even though no MAP column was involved.
Since read_blobs() can materialize large datasets, this O(rows) metadata
regression can add substantial GC pressure or trigger OOMs for existing
scalar-only callers. Could we keep the previous contiguous offset/slice path
for scalar columns and reserve target mappings for MAP entries, for example by
passing the MAP column set into this helper?
--
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]
