You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "thswlsqls (via GitHub)" <gi...@apache.org> on 2026/07/03 07:22:55 UTC

[PR] Arrow: Fix dict-encoded VARCHAR/VARBINARY read for direct ByteBuffers [iceberg]

thswlsqls opened a new pull request, #17055:
URL: https://github.com/apache/iceberg/pull/17055

   
   Closes #17053
   
   ## Summary
   
   - `VarWidthBinaryDictEncodedReader.nextVal()` called `buffer.array()`/`buffer.arrayOffset()` directly, which throws `UnsupportedOperationException` when the dictionary's `ByteBuffer` is not array-backed (e.g. direct).
   - Replaced this with `BaseVariableWidthVector.setSafe(int, ByteBuffer, int, int)`, which delegates to `ArrowBuf.setBytes` and safely copies both direct and heap buffers, so no `hasArray()` branch is needed.
   - Matches the same-module sibling readers that already guard this: `VectorizedPlainValuesReader.readBinary()`, `VectorizedDeltaLengthByteArrayValuesReader.readBinary()`, `VectorizedDeltaEncodedValuesReader.unpackMiniBlock()`.
   
   ## Testing done
   
   - Added `TestVectorizedDictionaryEncodedParquetValuesReader#varWidthBinaryDictEncodedReaderHandlesNonArrayBackedByteBuffer` — a custom `Dictionary` returns a `Binary` backed by a direct (non-array-backed) `ByteBuffer`; verified the test throws `UnsupportedOperationException` before this fix and passes after.
   - `./gradlew :iceberg-arrow:check` — 30 tests passed.
   
   


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Arrow: Fix dict-encoded VARCHAR/VARBINARY read for direct ByteBuffers [iceberg]

Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on code in PR #17055:
URL: https://github.com/apache/iceberg/pull/17055#discussion_r3535519372


##########
arrow/src/main/java/org/apache/iceberg/arrow/vectorized/parquet/VectorizedDictionaryEncodedParquetValuesReader.java:
##########
@@ -143,11 +143,7 @@ protected void nextVal(
         FieldVector vector, Dictionary dict, int idx, int currentVal, int typeWidth) {
       ByteBuffer buffer = dict.decodeToBinary(currentVal).toByteBuffer();
       ((BaseVariableWidthVector) vector)
-          .setSafe(
-              idx,
-              buffer.array(),
-              buffer.position() + buffer.arrayOffset(),
-              buffer.limit() - buffer.position());
+          .setSafe(idx, buffer, buffer.position(), buffer.remaining());

Review Comment:
   Would this have any performance implications?



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Arrow: Fix dict-encoded VARCHAR/VARBINARY read for direct ByteBuffers [iceberg]

Posted by "thswlsqls (via GitHub)" <gi...@apache.org>.
thswlsqls commented on code in PR #17055:
URL: https://github.com/apache/iceberg/pull/17055#discussion_r3562742839


##########
arrow/src/main/java/org/apache/iceberg/arrow/vectorized/parquet/VectorizedDictionaryEncodedParquetValuesReader.java:
##########
@@ -143,11 +143,7 @@ protected void nextVal(
         FieldVector vector, Dictionary dict, int idx, int currentVal, int typeWidth) {
       ByteBuffer buffer = dict.decodeToBinary(currentVal).toByteBuffer();
       ((BaseVariableWidthVector) vector)
-          .setSafe(
-              idx,
-              buffer.array(),
-              buffer.position() + buffer.arrayOffset(),
-              buffer.limit() - buffer.position());
+          .setSafe(idx, buffer, buffer.position(), buffer.remaining());

Review Comment:
   No meaningful difference — it's still a single copy of the same bytes. The `ByteBuffer` overload of `setSafe` copies `remaining()` bytes the same way; the old `buffer.array()` path just couldn't handle direct (non-array-backed) buffers, which is the bug this fixes.
   



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Arrow: Fix dict-encoded VARCHAR/VARBINARY read for direct ByteBuffers [iceberg]

Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on PR #17055:
URL: https://github.com/apache/iceberg/pull/17055#issuecomment-4957575801

   Merged to main.
   Thanks @thswlsqls for the PR!


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Arrow: Fix dict-encoded VARCHAR/VARBINARY read for direct ByteBuffers [iceberg]

Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary merged PR #17055:
URL: https://github.com/apache/iceberg/pull/17055


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org