You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "ddupg (via GitHub)" <gi...@apache.org> on 2026/09/20 03:12:37 UTC

[PR] GH-1302: Add support for the arrow.json extension type [arrow-java]

ddupg opened a new pull request, #1303:
URL: https://github.com/apache/arrow-java/pull/1303

   Adds `JsonType` and `JsonVector` for the canonical `arrow.json` extension, backed by `Utf8`, `LargeUtf8`, or `Utf8View`.
   
   Includes metadata handling, registration, and vector transfers that preserve the extension type. IPC loading and unloading now inspect the underlying vector when counting view buffers, so JSON backed by `Utf8View` can round-trip correctly.
   
   Tested with `mvn -pl vector -am test` on JDK 17 and bidirectional IPC checks with PyArrow 24.0.0 for all three storage types.
   
   Closes #1302.
   


-- 
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: github-unsubscribe@arrow.apache.org

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


Re: [PR] GH-1302: Add support for the arrow.json extension type [arrow-java]

Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre merged PR #1303:
URL: https://github.com/apache/arrow-java/pull/1303


-- 
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: github-unsubscribe@arrow.apache.org

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


Re: [PR] GH-1302: Add support for the arrow.json extension type [arrow-java]

Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on code in PR #1303:
URL: https://github.com/apache/arrow-java/pull/1303#discussion_r4173246804


##########
vector/src/main/java/org/apache/arrow/vector/VectorLoader.java:
##########
@@ -106,11 +106,15 @@ private void loadBuffers(
       Iterator<ArrowFieldNode> nodes,
       CompressionCodec codec,
       Iterator<Long> variadicBufferCounts) {
+    FieldVector storageVector = vector;
+    while (storageVector instanceof ExtensionTypeVector) {
+      storageVector = ((ExtensionTypeVector<?>) storageVector).getUnderlyingVector();
+    }
     checkArgument(nodes.hasNext(), "no more field nodes for field %s and vector %s", field, vector);
     ArrowFieldNode fieldNode = nodes.next();
-    // variadicBufferLayoutCount will be 0 for vectors of a type except BaseVariableWidthViewVector
+    // Only view storage has variadic buffers.
     long variadicBufferLayoutCount = 0;
-    if (vector instanceof BaseVariableWidthViewVector) {
+    if (storageVector instanceof BaseVariableWidthViewVector) {
       if (variadicBufferCounts.hasNext()) {

Review Comment:
   Same problem in the export direction: this now adds a variadic count for extension vectors on view storage, but `c/StructVectorLoader.loadBuffers` still checks the wrapper and never consumes it.
   
   `Data.exportVectorSchemaRoot` (and `ArrayStreamExporter`) on a root with a `JsonVector(Utf8View)` column then throws `IllegalArgumentException: not all nodes, buffers and variadicBufferCounts were consumed`. It fails even with zero variadic buffers, a case that succeeds on `main`.
   
   `StructVectorLoader` needs the same change as `VectorLoader`. Since all four loader/unloader classes have to agree on which vectors carry a variadic count, a single shared helper for the unwrap (for example a static storage-vector accessor on `ExtensionTypeVector`) would keep them in sync.



##########
vector/src/main/java/org/apache/arrow/vector/VectorLoader.java:
##########
@@ -106,11 +106,15 @@ private void loadBuffers(
       Iterator<ArrowFieldNode> nodes,
       CompressionCodec codec,
       Iterator<Long> variadicBufferCounts) {
+    FieldVector storageVector = vector;
+    while (storageVector instanceof ExtensionTypeVector) {
+      storageVector = ((ExtensionTypeVector<?>) storageVector).getUnderlyingVector();
+    }
     checkArgument(nodes.hasNext(), "no more field nodes for field %s and vector %s", field, vector);
     ArrowFieldNode fieldNode = nodes.next();
-    // variadicBufferLayoutCount will be 0 for vectors of a type except BaseVariableWidthViewVector
+    // Only view storage has variadic buffers.
     long variadicBufferLayoutCount = 0;
-    if (vector instanceof BaseVariableWidthViewVector) {
+    if (storageVector instanceof BaseVariableWidthViewVector) {

Review Comment:
   This now expects a `variadicBufferCounts` entry for extension vectors whose storage is a view vector, but `c/StructVectorUnloader` (which feeds this loader in `Data.importIntoVectorSchemaRoot`) still checks `vector instanceof BaseVariableWidthViewVector` on the wrapper, so it never emits one.
   
   Importing a C Data batch with an `arrow.json` or `OpaqueType` column on Utf8View/BinaryView storage now fails with `IlllegalStateException: No variadicBufferCounts available for BaseVariableWidthViewVector` when all values are inlined (<= 12 bytes) or the batch is empty. The same `OpaqueVector(Utf8View)` case loads on `main`, so this is a regression for existing extension types, not only for the new one. It also affects `ArrowArrayStreamReader.loadNextBatch` and the dataset `NativeScanner`.
   
   Could you apply the same unwrapping in `StructVectorUnloader`, and add a C Data round-trip test for an extension type on view storage? `RoundtripTest.testExtensionTypeVector` only covers `UuidType`.



##########
vector/src/main/java/org/apache/arrow/vector/extension/JsonVector.java:
##########
@@ -0,0 +1,126 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.arrow.vector.extension;
+
+import org.apache.arrow.memory.BufferAllocator;
+import org.apache.arrow.memory.util.hash.ArrowBufHasher;
+import org.apache.arrow.vector.ExtensionTypeVector;
+import org.apache.arrow.vector.FieldVector;
+import org.apache.arrow.vector.ValueIterableVector;
+import org.apache.arrow.vector.ValueVector;
+import org.apache.arrow.vector.types.pojo.Field;
+import org.apache.arrow.vector.util.CallBack;
+import org.apache.arrow.vector.util.Text;
+import org.apache.arrow.vector.util.TransferPair;
+
+/**
+ * A JSON extension vector backed by a string vector.
+ *
+ * <p>Use {@link Field#createVector(BufferAllocator)} with a {@link JsonType} field to create an
+ * instance. Write UTF-8 JSON through {@link #getUnderlyingVector()}; values are not parsed or
+ * validated.
+ */
+public class JsonVector extends ExtensionTypeVector<FieldVector>

Review Comment:
   `ExtensionTypeVector` does not delegate `exportCDataBuffers` / `getExportedCDataBufferCount` to the storage vector, so `JsonVector` inherits the `FieldVector` defaults. For Utf8View storage that means `Data.exportVector` exports 3 buffers (validity, views, data) where `ViewVarCharVector` exports 4, i.e. the trailing variadic-sizes buffer is missing.
   
   On the import side, `BufferImportTypeVisitor.visitVariableWidthView` takes the last buffer as the sizes buffer and computes the data buffer count as `buffers.length - 3`, so a consumer would misread the data buffer as sizes.
   
   Delegating both methods to `getUnderlyingVector()` would fix it, ideally in `ExtensionTypeVector` so `OpaqueVector` on view storage gets it too.



-- 
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: github-unsubscribe@arrow.apache.org

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


Re: [PR] GH-1302: Add support for the arrow.json extension type [arrow-java]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #1303:
URL: https://github.com/apache/arrow-java/pull/1303#issuecomment-5747251710

   
   <!-- label_helper_comment -->
   Thank you for opening a pull request!
   
   Please label the PR with one or more of:
   
   - bug-fix
   - chore
   - dependencies
   - documentation
   - enhancement
   
   Also, add the 'breaking-change' label if appropriate.
   
   See [CONTRIBUTING.md](https://github.com/apache/arrow-java/blob/main/CONTRIBUTING.md) for details.
   


-- 
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: github-unsubscribe@arrow.apache.org

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


Re: [PR] GH-1302: Add support for the arrow.json extension type [arrow-java]

Posted by "ddupg (via GitHub)" <gi...@apache.org>.
ddupg commented on PR #1303:
URL: https://github.com/apache/arrow-java/pull/1303#issuecomment-5975523362

   @jbonofre Thanks for the review! Fixed as suggested, with a shared unwrapping helper and C Data export delegation. Added roundtrip tests for JSON and Opaque view vectors.


-- 
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: github-unsubscribe@arrow.apache.org

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