github-actions[bot] commented on code in PR #69658:
URL: https://github.com/apache/doris/pull/69658#discussion_r4384797785
##########
fe/be-java-extensions/paimon-connector/src/main/java/org/apache/doris/paimon/PaimonColumnValue.java:
##########
@@ -291,6 +310,53 @@ public void unpackStruct(List<Integer> structFieldIndex,
List<ColumnValue> value
}
}
+ @Override
+ public FileValue getFile() {
+ Blob blob = record.getBlob(idx);
Review Comment:
[P2] Supply FileIO to nested columnar BLOB values
A valid Paimon 1.4.2 Parquet column `payload ROW<image BLOB>` can store an
external BlobDescriptor. This projection is routed to JNI, but `unpackStruct()`
obtains a child `ColumnarRow` from the SDK's `CastedRowColumnVector.getRow()`,
which constructs it without FileIO. Calling `record.getBlob(idx)` here then
throws `IllegalStateException("FileIO is null, cannot read blob data from
uri!")` before either output mode is handled. Thus non-NULL nested BLOBs fail
in both inline and descriptor mode. The added nested test uses `GenericRow`,
which bypasses this path. Carry the effective table FileIO into nested columnar
rows, or decode their descriptor bytes with that FileIO, and cover a real
Parquet ROW<BLOB> scan. The pinned SDK behavior is visible in
[CastedRowColumnVector](https://github.com/apache/paimon/blob/release-1.4.2/paimon-common/src/main/java/org/apache/paimon/data/columnar/heap/CastedRowColumnVector.java)
and [ColumnarRow](https://github.com/apache/paimon/blob/release-1.4.2/p
aimon-common/src/main/java/org/apache/paimon/data/columnar/ColumnarRow.java).
##########
fe/be-java-extensions/java-common/src/main/java/org/apache/doris/common/jni/vec/VectorColumn.java:
##########
@@ -1544,6 +1551,100 @@ public HashMap<String, Object>[] getStructColumn(int
start, int end) {
return result;
}
+ /** Append FILE's canonical six children. Payload bytes are copied into
off-heap storage. */
+ public int appendFile(FileValue file) {
+ if (file == null) {
+ return appendNull(Type.FILE);
+ }
+ reserve(appendIndex + 1);
+ VectorColumn uriColumn = childColumns[FileValue.URI_FIELD_INDEX];
+ if (file.getUri() == null) {
+ uriColumn.appendNull(Type.STRING);
+ } else {
+ uriColumn.appendStringAndOffset(file.getUri());
+ }
+
+ VectorColumn offsetColumn = childColumns[FileValue.OFFSET_FIELD_INDEX];
+ if (file.getOffset() == null) {
+ offsetColumn.appendNull(Type.BIGINT);
+ } else {
+ offsetColumn.appendLong(file.getOffset());
+ }
+
+ VectorColumn sizeColumn = childColumns[FileValue.SIZE_FIELD_INDEX];
+ if (file.getSize() == null) {
+ sizeColumn.appendNull(Type.BIGINT);
+ } else {
+ sizeColumn.appendLong(file.getSize());
+ }
+
+ VectorColumn contentTypeColumn =
childColumns[FileValue.CONTENT_TYPE_FIELD_INDEX];
+ if (file.getContentType() == null) {
+ contentTypeColumn.appendNull(Type.STRING);
+ } else {
+ contentTypeColumn.appendStringAndOffset(file.getContentType());
+ }
+
+ VectorColumn checksumColumn =
childColumns[FileValue.CHECKSUM_FIELD_INDEX];
+ if (file.getChecksum() == null) {
+ checksumColumn.appendNull(Type.STRING);
+ } else {
+ checksumColumn.appendStringAndOffset(file.getChecksum());
+ }
+
+ VectorColumn inlineColumn = childColumns[FileValue.INLINE_FIELD_INDEX];
+ if (file.getInline() == null) {
+ inlineColumn.appendNull(Type.VARBINARY);
Review Comment:
[P2] Avoid zero-byte allocations for FILE inline placeholders
For every descriptor, this branch calls `appendNull(VARBINARY)`, which
reaches scalar `appendVarbinary(new byte[0])` and `OffHeap.allocateMemory(0)`.
In production that invokes native `malloc(0)`; Doris's jemalloc 5.3.0 returns a
nonzero allocation, but `freeVarbinaryData()` only frees entries whose length
is greater than zero. Closing/releasing each batch therefore loses that
allocation permanently. SQL NULL FILEs and empty inline payloads take the same
path, so repeated descriptor scans leak memory in proportion to row count.
Avoid allocating for zero-length payloads and store `[0,0]`, as the existing
batch VARBINARY writer does, or free every owned nonzero pointer regardless of
length. The new tests use `OffHeap.setTesting()` and do not exercise this
production allocator behavior; jemalloc's [zero-size allocation
test](https://github.com/jemalloc/jemalloc/blob/5.3.0/test/integration/malloc.c)
confirms the allocation requires freeing.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
