You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "tmater (via GitHub)" <gi...@apache.org> on 2026/01/07 09:23:24 UTC
[PR] GH-946: Add Variant extension type support [arrow-java]
tmater opened a new pull request, #947:
URL: https://github.com/apache/arrow-java/pull/947
Implements VariantType extension type with VariantVector for storing variant data with metadata and value buffers. Includes reader/writer implementations and comprehensive test coverage.
## What's Changed
- Core Types: VariantType, VariantVector, and holder classes
- Reader/Writer Support: Reader/writer implementations
Closes #GH-946.
--
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-946: Add Variant extension type support [arrow-java]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre merged PR #947:
URL: https://github.com/apache/arrow-java/pull/947
--
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-946: Add Variant extension type support [arrow-java]
Posted by "gszadovszky (via GitHub)" <gi...@apache.org>.
gszadovszky commented on code in PR #947:
URL: https://github.com/apache/arrow-java/pull/947#discussion_r2735652361
##########
vector/src/main/java/org/apache/arrow/vector/extension/VariantVector.java:
##########
@@ -0,0 +1,342 @@
+/*
+ * 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 java.nio.ByteBuffer;
+import java.util.List;
+import org.apache.arrow.memory.ArrowBuf;
+import org.apache.arrow.memory.BufferAllocator;
+import org.apache.arrow.memory.util.hash.ArrowBufHasher;
+import org.apache.arrow.vector.BitVectorHelper;
+import org.apache.arrow.vector.ExtensionTypeVector;
+import org.apache.arrow.vector.FieldVector;
+import org.apache.arrow.vector.ValueVector;
+import org.apache.arrow.vector.VarBinaryVector;
+import org.apache.arrow.vector.complex.AbstractStructVector;
+import org.apache.arrow.vector.complex.StructVector;
+import org.apache.arrow.vector.complex.reader.FieldReader;
+import org.apache.arrow.vector.holders.NullableVariantHolder;
+import org.apache.arrow.vector.holders.VariantHolder;
+import org.apache.arrow.vector.types.pojo.ArrowType;
+import org.apache.arrow.vector.types.pojo.ArrowType.Binary;
+import org.apache.arrow.vector.types.pojo.Field;
+import org.apache.arrow.vector.types.pojo.FieldType;
+import org.apache.arrow.vector.util.CallBack;
+import org.apache.arrow.vector.util.TransferPair;
+import org.apache.arrow.vector.variant.Variant;
+
+public class VariantVector extends ExtensionTypeVector<StructVector> {
Review Comment:
Referencing `VariantType` here would help the user understanding what this vector is for.
##########
flight/flight-sql-jdbc-driver/pom.xml:
##########
@@ -93,6 +93,11 @@ under the License.
<exclude>com.google.errorprone:error_prone_annotations</exclude>
<exclude>com.google.code.findbugs:jsr305</exclude>
<exclude>com.google.j2objc:j2objc-annotations</exclude>
+ <!-- Parquet transitive dependencies (from arrow-variant) that contain pre-shaded classes -->
+ <exclude>org.apache.parquet:parquet-common</exclude>
+ <exclude>org.apache.parquet:parquet-column</exclude>
+ <exclude>org.apache.parquet:parquet-encoding</exclude>
+ <exclude>org.apache.parquet:parquet-format-structures</exclude>
Review Comment:
This is not required if the variant is separated since flight does not depend on that module.
##########
arrow-variant/src/main/java/org/apache/arrow/vector/variant/Variant.java:
##########
@@ -0,0 +1,205 @@
+/*
+ * 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.variant;
+
+import java.math.BigDecimal;
+import java.nio.ByteBuffer;
+import java.util.Objects;
+import java.util.UUID;
+import org.apache.arrow.memory.ArrowBuf;
+
+/**
+ * Wrapper around parquet-variant's Variant implementation.
+ *
+ * <p>This wrapper exists to isolate the parquet-variant dependency from Arrow's public API,
+ * allowing the vector module to expose variant functionality without requiring users to depend on
+ * parquet-variant directly. It also ensures that nested variant values (from arrays and objects)
+ * are consistently wrapped.
+ */
+public class Variant {
+
+ private final org.apache.parquet.variant.Variant delegate;
+
+ /** Creates a Variant from raw metadata and value byte arrays. */
+ public Variant(byte[] metadata, byte[] value) {
+ this.delegate = new org.apache.parquet.variant.Variant(value, metadata);
+ }
+
+ /** Creates a Variant by copying data from ArrowBuf instances. */
+ public Variant(
+ ArrowBuf metadataBuffer,
+ int metadataStart,
+ int metadataEnd,
+ ArrowBuf valueBuffer,
+ int valueStart,
+ int valueEnd) {
+ byte[] metadata = new byte[metadataEnd - metadataStart];
+ byte[] value = new byte[valueEnd - valueStart];
+ metadataBuffer.getBytes(metadataStart, metadata);
+ valueBuffer.getBytes(valueStart, value);
+ this.delegate = new org.apache.parquet.variant.Variant(value, metadata);
+ }
+
+ private Variant(org.apache.parquet.variant.Variant delegate) {
+ this.delegate = delegate;
+ }
+
+ public ByteBuffer getValueBuffer() {
+ return delegate.getValueBuffer();
+ }
+
+ public ByteBuffer getMetadataBuffer() {
+ return delegate.getMetadataBuffer();
+ }
+
Review Comment:
I don't think we should expose these to the user.
##########
vector/src/main/java/module-info.java:
##########
@@ -46,6 +46,7 @@
requires jdk.unsupported;
requires org.apache.arrow.format;
requires org.apache.arrow.memory.core;
+ requires org.apache.arrow.variant;
Review Comment:
The `vector` module should not depend on `arrow-variant'
##########
vector/src/main/java/org/apache/arrow/vector/extension/VariantType.java:
##########
@@ -0,0 +1,88 @@
+/*
+ * 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.vector.FieldVector;
+import org.apache.arrow.vector.ValueVector;
+import org.apache.arrow.vector.complex.impl.VariantWriterImpl;
+import org.apache.arrow.vector.complex.writer.FieldWriter;
+import org.apache.arrow.vector.types.pojo.ArrowType;
+import org.apache.arrow.vector.types.pojo.ArrowType.ExtensionType;
+import org.apache.arrow.vector.types.pojo.ExtensionTypeRegistry;
+import org.apache.arrow.vector.types.pojo.FieldType;
+
+public final class VariantType extends ExtensionType {
Review Comment:
Maybe adding some comments about the type referencing the related Parquet spec would help understanding this. It should also be mentioned that it is for the variant binary format and does not handle shredded data.
##########
vector/src/test/java/org/apache/arrow/vector/TestListVector.java:
##########
Review Comment:
See above
##########
vector/src/test/java/org/apache/arrow/vector/types/pojo/TestExtensionType.java:
##########
Review Comment:
Move these tests to `TestVariant`.
##########
vector/pom.xml:
##########
@@ -37,6 +37,17 @@ under the License.
<groupId>org.apache.arrow</groupId>
<artifactId>arrow-memory-core</artifactId>
</dependency>
+ <dependency>
+ <groupId>org.apache.arrow</groupId>
+ <artifactId>arrow-variant</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.arrow</groupId>
+ <artifactId>arrow-variant</artifactId>
+ <version>${project.version}</version>
+ <type>test-jar</type>
+ <scope>test</scope>
+ </dependency>
Review Comment:
`vector` should not depend on `arrow-variant`
--
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-946: Add Variant extension type support [arrow-java]
Posted by "gszadovszky (via GitHub)" <gi...@apache.org>.
gszadovszky commented on code in PR #947:
URL: https://github.com/apache/arrow-java/pull/947#discussion_r2715636532
##########
vector/src/main/java/org/apache/arrow/vector/extension/VariantVector.java:
##########
@@ -0,0 +1,306 @@
+/*
+ * 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 java.util.List;
+import org.apache.arrow.memory.BufferAllocator;
+import org.apache.arrow.memory.util.hash.ArrowBufHasher;
+import org.apache.arrow.vector.BitVectorHelper;
+import org.apache.arrow.vector.ExtensionTypeVector;
+import org.apache.arrow.vector.FieldVector;
+import org.apache.arrow.vector.ValueVector;
+import org.apache.arrow.vector.VarBinaryVector;
+import org.apache.arrow.vector.complex.AbstractStructVector;
+import org.apache.arrow.vector.complex.StructVector;
+import org.apache.arrow.vector.complex.reader.FieldReader;
+import org.apache.arrow.vector.holders.NullableVariantHolder;
+import org.apache.arrow.vector.holders.VariantHolder;
+import org.apache.arrow.vector.types.pojo.ArrowType;
+import org.apache.arrow.vector.types.pojo.ArrowType.Binary;
+import org.apache.arrow.vector.types.pojo.Field;
+import org.apache.arrow.vector.types.pojo.FieldType;
+import org.apache.arrow.vector.util.CallBack;
+import org.apache.arrow.vector.util.TransferPair;
+
+public class VariantVector extends ExtensionTypeVector<StructVector> {
+
+ public static final String METADATA_VECTOR_NAME = "metadata";
+ public static final String VALUE_VECTOR_NAME = "value";
+
+ private final Field rootField;
+
+ /**
+ * Constructs a new VariantVector with the given name and allocator.
+ *
+ * @param name the name of the vector
+ * @param allocator the buffer allocator for memory management
+ */
+ public VariantVector(String name, BufferAllocator allocator) {
+ super(
+ name,
+ allocator,
+ new StructVector(
+ name,
+ allocator,
+ FieldType.nullable(ArrowType.Struct.INSTANCE),
+ null,
+ AbstractStructVector.ConflictPolicy.CONFLICT_ERROR,
+ false));
+ rootField = createVariantField(name);
+ ((FieldVector) this.getUnderlyingVector())
+ .initializeChildrenFromFields(rootField.getChildren());
+ }
+
+ /**
+ * Creates a new VariantVector with the given name. The Variant Field schema has to be the same
+ * everywhere, otherwise ArrowBuffer loading might fail during serialization/deserialization and
+ * schema mismatches can occur. This includes CompleteType's VARIANT and VARIANT_REQUIRED types.
+ */
+ public static Field createVariantField(String name) {
+ return new Field(
+ name, new FieldType(true, VariantType.INSTANCE, null), createVariantChildFields());
+ }
+
+ /**
+ * Creates the child fields for the VariantVector. Metadata vector will be index 0 and value
+ * vector will be index 1.
+ */
+ public static List<Field> createVariantChildFields() {
+ return List.of(
+ new Field(METADATA_VECTOR_NAME, new FieldType(false, Binary.INSTANCE, null), null),
+ new Field(VALUE_VECTOR_NAME, new FieldType(false, Binary.INSTANCE, null), null));
+ }
+
+ @Override
+ public void initializeChildrenFromFields(List<Field> children) {
+ // No-op, as children are initialized in the constructor
+ }
+
+ @Override
+ public Field getField() {
+ return rootField;
+ }
+
+ public VarBinaryVector getMetadataVector() {
+ return getUnderlyingVector().getChild(METADATA_VECTOR_NAME, VarBinaryVector.class);
+ }
+
+ public VarBinaryVector getValueVector() {
+ return getUnderlyingVector().getChild(VALUE_VECTOR_NAME, VarBinaryVector.class);
+ }
+
+ @Override
+ public TransferPair makeTransferPair(ValueVector target) {
+ return new VariantTransferPair(this, (VariantVector) target);
+ }
+
+ @Override
+ public TransferPair getTransferPair(Field field, BufferAllocator allocator) {
+ return new VariantTransferPair(this, new VariantVector(field.getName(), allocator));
+ }
+
+ @Override
+ public TransferPair getTransferPair(Field field, BufferAllocator allocator, CallBack callBack) {
+ return getTransferPair(field, allocator);
+ }
+
+ @Override
+ public TransferPair getTransferPair(String ref, BufferAllocator allocator) {
+ return new VariantTransferPair(this, new VariantVector(ref, allocator));
+ }
+
+ @Override
+ public TransferPair getTransferPair(String ref, BufferAllocator allocator, CallBack callBack) {
+ return getTransferPair(ref, allocator);
+ }
+
+ @Override
+ public TransferPair getTransferPair(BufferAllocator allocator) {
+ return getTransferPair(this.getField().getName(), allocator);
+ }
+
+ @Override
+ public void copyFrom(int fromIndex, int thisIndex, ValueVector from) {
+ getUnderlyingVector()
+ .copyFrom(fromIndex, thisIndex, ((VariantVector) from).getUnderlyingVector());
+ }
+
+ @Override
+ public void copyFromSafe(int fromIndex, int thisIndex, ValueVector from) {
+ getUnderlyingVector()
+ .copyFromSafe(fromIndex, thisIndex, ((VariantVector) from).getUnderlyingVector());
+ }
+
+ @Override
+ public Object getObject(int index) {
+ return getUnderlyingVector().getObject(index);
+ }
Review Comment:
I'm wondering if the variant representation of [parquet-java](https://github.com/apache/parquet-java/blob/master/parquet-variant) would be suitable here. That is a java object backed by nio `ByteBuffer`s. It would be beneficial as arrow-java would not need to maintain/test the related code. Meanwhile, it does not support returning "rich" objects (e.g. timestamps) but the primitive representations only (e.g. long for a timestamp). I'm not sure how it would fit the current concept.
Alternatively, we would be able to wrap the parquet-java `Variant` object and extend it with those rich types the different Arrow vectors support (e.g. `LocalDateTime` for a timestamp nano value).
--
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-946: Add Variant extension type support [arrow-java]
Posted by "gszadovszky (via GitHub)" <gi...@apache.org>.
gszadovszky commented on code in PR #947:
URL: https://github.com/apache/arrow-java/pull/947#discussion_r2715912269
##########
vector/src/main/java/org/apache/arrow/vector/extension/VariantVector.java:
##########
@@ -0,0 +1,306 @@
+/*
+ * 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 java.util.List;
+import org.apache.arrow.memory.BufferAllocator;
+import org.apache.arrow.memory.util.hash.ArrowBufHasher;
+import org.apache.arrow.vector.BitVectorHelper;
+import org.apache.arrow.vector.ExtensionTypeVector;
+import org.apache.arrow.vector.FieldVector;
+import org.apache.arrow.vector.ValueVector;
+import org.apache.arrow.vector.VarBinaryVector;
+import org.apache.arrow.vector.complex.AbstractStructVector;
+import org.apache.arrow.vector.complex.StructVector;
+import org.apache.arrow.vector.complex.reader.FieldReader;
+import org.apache.arrow.vector.holders.NullableVariantHolder;
+import org.apache.arrow.vector.holders.VariantHolder;
+import org.apache.arrow.vector.types.pojo.ArrowType;
+import org.apache.arrow.vector.types.pojo.ArrowType.Binary;
+import org.apache.arrow.vector.types.pojo.Field;
+import org.apache.arrow.vector.types.pojo.FieldType;
+import org.apache.arrow.vector.util.CallBack;
+import org.apache.arrow.vector.util.TransferPair;
+
+public class VariantVector extends ExtensionTypeVector<StructVector> {
+
+ public static final String METADATA_VECTOR_NAME = "metadata";
+ public static final String VALUE_VECTOR_NAME = "value";
+
+ private final Field rootField;
+
+ /**
+ * Constructs a new VariantVector with the given name and allocator.
+ *
+ * @param name the name of the vector
+ * @param allocator the buffer allocator for memory management
+ */
+ public VariantVector(String name, BufferAllocator allocator) {
+ super(
+ name,
+ allocator,
+ new StructVector(
+ name,
+ allocator,
+ FieldType.nullable(ArrowType.Struct.INSTANCE),
+ null,
+ AbstractStructVector.ConflictPolicy.CONFLICT_ERROR,
+ false));
+ rootField = createVariantField(name);
+ ((FieldVector) this.getUnderlyingVector())
+ .initializeChildrenFromFields(rootField.getChildren());
+ }
+
+ /**
+ * Creates a new VariantVector with the given name. The Variant Field schema has to be the same
+ * everywhere, otherwise ArrowBuffer loading might fail during serialization/deserialization and
+ * schema mismatches can occur. This includes CompleteType's VARIANT and VARIANT_REQUIRED types.
+ */
+ public static Field createVariantField(String name) {
+ return new Field(
+ name, new FieldType(true, VariantType.INSTANCE, null), createVariantChildFields());
+ }
+
+ /**
+ * Creates the child fields for the VariantVector. Metadata vector will be index 0 and value
+ * vector will be index 1.
+ */
+ public static List<Field> createVariantChildFields() {
+ return List.of(
+ new Field(METADATA_VECTOR_NAME, new FieldType(false, Binary.INSTANCE, null), null),
+ new Field(VALUE_VECTOR_NAME, new FieldType(false, Binary.INSTANCE, null), null));
+ }
+
+ @Override
+ public void initializeChildrenFromFields(List<Field> children) {
+ // No-op, as children are initialized in the constructor
+ }
+
+ @Override
+ public Field getField() {
+ return rootField;
+ }
+
+ public VarBinaryVector getMetadataVector() {
+ return getUnderlyingVector().getChild(METADATA_VECTOR_NAME, VarBinaryVector.class);
+ }
+
+ public VarBinaryVector getValueVector() {
+ return getUnderlyingVector().getChild(VALUE_VECTOR_NAME, VarBinaryVector.class);
+ }
+
+ @Override
+ public TransferPair makeTransferPair(ValueVector target) {
+ return new VariantTransferPair(this, (VariantVector) target);
+ }
+
+ @Override
+ public TransferPair getTransferPair(Field field, BufferAllocator allocator) {
+ return new VariantTransferPair(this, new VariantVector(field.getName(), allocator));
+ }
+
+ @Override
+ public TransferPair getTransferPair(Field field, BufferAllocator allocator, CallBack callBack) {
+ return getTransferPair(field, allocator);
+ }
+
+ @Override
+ public TransferPair getTransferPair(String ref, BufferAllocator allocator) {
+ return new VariantTransferPair(this, new VariantVector(ref, allocator));
+ }
+
+ @Override
+ public TransferPair getTransferPair(String ref, BufferAllocator allocator, CallBack callBack) {
+ return getTransferPair(ref, allocator);
+ }
+
+ @Override
+ public TransferPair getTransferPair(BufferAllocator allocator) {
+ return getTransferPair(this.getField().getName(), allocator);
+ }
+
+ @Override
+ public void copyFrom(int fromIndex, int thisIndex, ValueVector from) {
+ getUnderlyingVector()
+ .copyFrom(fromIndex, thisIndex, ((VariantVector) from).getUnderlyingVector());
+ }
+
+ @Override
+ public void copyFromSafe(int fromIndex, int thisIndex, ValueVector from) {
+ getUnderlyingVector()
+ .copyFromSafe(fromIndex, thisIndex, ((VariantVector) from).getUnderlyingVector());
+ }
+
+ @Override
+ public Object getObject(int index) {
+ return getUnderlyingVector().getObject(index);
+ }
Review Comment:
Sorry, I've misinterpreted @jbonofre's suggestion. I wanted to reference the `parquet-java`'s `parquet-variant` module. That contains only 10 prod classes. But right, it has a dependency on other `parquet-java` modules as well. So, we have two options:
* Create the separate `arrow-variant` module that contains any variant related classes, including the ones already included in this PR.
* Duplicate the variant related code from `parquet-java`, and implement our own classes based on it in the existing `vector` module.
I do not have a strong opinion which one would be the best choice for Arrow.
--
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-946: Add Variant extension type support [arrow-java]
Posted by "gszadovszky (via GitHub)" <gi...@apache.org>.
gszadovszky commented on code in PR #947:
URL: https://github.com/apache/arrow-java/pull/947#discussion_r2716077038
##########
vector/src/main/java/org/apache/arrow/vector/extension/VariantVector.java:
##########
@@ -0,0 +1,306 @@
+/*
+ * 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 java.util.List;
+import org.apache.arrow.memory.BufferAllocator;
+import org.apache.arrow.memory.util.hash.ArrowBufHasher;
+import org.apache.arrow.vector.BitVectorHelper;
+import org.apache.arrow.vector.ExtensionTypeVector;
+import org.apache.arrow.vector.FieldVector;
+import org.apache.arrow.vector.ValueVector;
+import org.apache.arrow.vector.VarBinaryVector;
+import org.apache.arrow.vector.complex.AbstractStructVector;
+import org.apache.arrow.vector.complex.StructVector;
+import org.apache.arrow.vector.complex.reader.FieldReader;
+import org.apache.arrow.vector.holders.NullableVariantHolder;
+import org.apache.arrow.vector.holders.VariantHolder;
+import org.apache.arrow.vector.types.pojo.ArrowType;
+import org.apache.arrow.vector.types.pojo.ArrowType.Binary;
+import org.apache.arrow.vector.types.pojo.Field;
+import org.apache.arrow.vector.types.pojo.FieldType;
+import org.apache.arrow.vector.util.CallBack;
+import org.apache.arrow.vector.util.TransferPair;
+
+public class VariantVector extends ExtensionTypeVector<StructVector> {
+
+ public static final String METADATA_VECTOR_NAME = "metadata";
+ public static final String VALUE_VECTOR_NAME = "value";
+
+ private final Field rootField;
+
+ /**
+ * Constructs a new VariantVector with the given name and allocator.
+ *
+ * @param name the name of the vector
+ * @param allocator the buffer allocator for memory management
+ */
+ public VariantVector(String name, BufferAllocator allocator) {
+ super(
+ name,
+ allocator,
+ new StructVector(
+ name,
+ allocator,
+ FieldType.nullable(ArrowType.Struct.INSTANCE),
+ null,
+ AbstractStructVector.ConflictPolicy.CONFLICT_ERROR,
+ false));
+ rootField = createVariantField(name);
+ ((FieldVector) this.getUnderlyingVector())
+ .initializeChildrenFromFields(rootField.getChildren());
+ }
+
+ /**
+ * Creates a new VariantVector with the given name. The Variant Field schema has to be the same
+ * everywhere, otherwise ArrowBuffer loading might fail during serialization/deserialization and
+ * schema mismatches can occur. This includes CompleteType's VARIANT and VARIANT_REQUIRED types.
+ */
+ public static Field createVariantField(String name) {
+ return new Field(
+ name, new FieldType(true, VariantType.INSTANCE, null), createVariantChildFields());
+ }
+
+ /**
+ * Creates the child fields for the VariantVector. Metadata vector will be index 0 and value
+ * vector will be index 1.
+ */
+ public static List<Field> createVariantChildFields() {
+ return List.of(
+ new Field(METADATA_VECTOR_NAME, new FieldType(false, Binary.INSTANCE, null), null),
+ new Field(VALUE_VECTOR_NAME, new FieldType(false, Binary.INSTANCE, null), null));
+ }
+
+ @Override
+ public void initializeChildrenFromFields(List<Field> children) {
+ // No-op, as children are initialized in the constructor
+ }
+
+ @Override
+ public Field getField() {
+ return rootField;
+ }
+
+ public VarBinaryVector getMetadataVector() {
+ return getUnderlyingVector().getChild(METADATA_VECTOR_NAME, VarBinaryVector.class);
+ }
+
+ public VarBinaryVector getValueVector() {
+ return getUnderlyingVector().getChild(VALUE_VECTOR_NAME, VarBinaryVector.class);
+ }
+
+ @Override
+ public TransferPair makeTransferPair(ValueVector target) {
+ return new VariantTransferPair(this, (VariantVector) target);
+ }
+
+ @Override
+ public TransferPair getTransferPair(Field field, BufferAllocator allocator) {
+ return new VariantTransferPair(this, new VariantVector(field.getName(), allocator));
+ }
+
+ @Override
+ public TransferPair getTransferPair(Field field, BufferAllocator allocator, CallBack callBack) {
+ return getTransferPair(field, allocator);
+ }
+
+ @Override
+ public TransferPair getTransferPair(String ref, BufferAllocator allocator) {
+ return new VariantTransferPair(this, new VariantVector(ref, allocator));
+ }
+
+ @Override
+ public TransferPair getTransferPair(String ref, BufferAllocator allocator, CallBack callBack) {
+ return getTransferPair(ref, allocator);
+ }
+
+ @Override
+ public TransferPair getTransferPair(BufferAllocator allocator) {
+ return getTransferPair(this.getField().getName(), allocator);
+ }
+
+ @Override
+ public void copyFrom(int fromIndex, int thisIndex, ValueVector from) {
+ getUnderlyingVector()
+ .copyFrom(fromIndex, thisIndex, ((VariantVector) from).getUnderlyingVector());
+ }
+
+ @Override
+ public void copyFromSafe(int fromIndex, int thisIndex, ValueVector from) {
+ getUnderlyingVector()
+ .copyFromSafe(fromIndex, thisIndex, ((VariantVector) from).getUnderlyingVector());
+ }
+
+ @Override
+ public Object getObject(int index) {
+ return getUnderlyingVector().getObject(index);
+ }
Review Comment:
Sounds good to me. @tmater, could you update your PR accordingly?
--
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-946: Add Variant extension type support [arrow-java]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on code in PR #947:
URL: https://github.com/apache/arrow-java/pull/947#discussion_r2715938954
##########
vector/src/main/java/org/apache/arrow/vector/extension/VariantVector.java:
##########
@@ -0,0 +1,306 @@
+/*
+ * 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 java.util.List;
+import org.apache.arrow.memory.BufferAllocator;
+import org.apache.arrow.memory.util.hash.ArrowBufHasher;
+import org.apache.arrow.vector.BitVectorHelper;
+import org.apache.arrow.vector.ExtensionTypeVector;
+import org.apache.arrow.vector.FieldVector;
+import org.apache.arrow.vector.ValueVector;
+import org.apache.arrow.vector.VarBinaryVector;
+import org.apache.arrow.vector.complex.AbstractStructVector;
+import org.apache.arrow.vector.complex.StructVector;
+import org.apache.arrow.vector.complex.reader.FieldReader;
+import org.apache.arrow.vector.holders.NullableVariantHolder;
+import org.apache.arrow.vector.holders.VariantHolder;
+import org.apache.arrow.vector.types.pojo.ArrowType;
+import org.apache.arrow.vector.types.pojo.ArrowType.Binary;
+import org.apache.arrow.vector.types.pojo.Field;
+import org.apache.arrow.vector.types.pojo.FieldType;
+import org.apache.arrow.vector.util.CallBack;
+import org.apache.arrow.vector.util.TransferPair;
+
+public class VariantVector extends ExtensionTypeVector<StructVector> {
+
+ public static final String METADATA_VECTOR_NAME = "metadata";
+ public static final String VALUE_VECTOR_NAME = "value";
+
+ private final Field rootField;
+
+ /**
+ * Constructs a new VariantVector with the given name and allocator.
+ *
+ * @param name the name of the vector
+ * @param allocator the buffer allocator for memory management
+ */
+ public VariantVector(String name, BufferAllocator allocator) {
+ super(
+ name,
+ allocator,
+ new StructVector(
+ name,
+ allocator,
+ FieldType.nullable(ArrowType.Struct.INSTANCE),
+ null,
+ AbstractStructVector.ConflictPolicy.CONFLICT_ERROR,
+ false));
+ rootField = createVariantField(name);
+ ((FieldVector) this.getUnderlyingVector())
+ .initializeChildrenFromFields(rootField.getChildren());
+ }
+
+ /**
+ * Creates a new VariantVector with the given name. The Variant Field schema has to be the same
+ * everywhere, otherwise ArrowBuffer loading might fail during serialization/deserialization and
+ * schema mismatches can occur. This includes CompleteType's VARIANT and VARIANT_REQUIRED types.
+ */
+ public static Field createVariantField(String name) {
+ return new Field(
+ name, new FieldType(true, VariantType.INSTANCE, null), createVariantChildFields());
+ }
+
+ /**
+ * Creates the child fields for the VariantVector. Metadata vector will be index 0 and value
+ * vector will be index 1.
+ */
+ public static List<Field> createVariantChildFields() {
+ return List.of(
+ new Field(METADATA_VECTOR_NAME, new FieldType(false, Binary.INSTANCE, null), null),
+ new Field(VALUE_VECTOR_NAME, new FieldType(false, Binary.INSTANCE, null), null));
+ }
+
+ @Override
+ public void initializeChildrenFromFields(List<Field> children) {
+ // No-op, as children are initialized in the constructor
+ }
+
+ @Override
+ public Field getField() {
+ return rootField;
+ }
+
+ public VarBinaryVector getMetadataVector() {
+ return getUnderlyingVector().getChild(METADATA_VECTOR_NAME, VarBinaryVector.class);
+ }
+
+ public VarBinaryVector getValueVector() {
+ return getUnderlyingVector().getChild(VALUE_VECTOR_NAME, VarBinaryVector.class);
+ }
+
+ @Override
+ public TransferPair makeTransferPair(ValueVector target) {
+ return new VariantTransferPair(this, (VariantVector) target);
+ }
+
+ @Override
+ public TransferPair getTransferPair(Field field, BufferAllocator allocator) {
+ return new VariantTransferPair(this, new VariantVector(field.getName(), allocator));
+ }
+
+ @Override
+ public TransferPair getTransferPair(Field field, BufferAllocator allocator, CallBack callBack) {
+ return getTransferPair(field, allocator);
+ }
+
+ @Override
+ public TransferPair getTransferPair(String ref, BufferAllocator allocator) {
+ return new VariantTransferPair(this, new VariantVector(ref, allocator));
+ }
+
+ @Override
+ public TransferPair getTransferPair(String ref, BufferAllocator allocator, CallBack callBack) {
+ return getTransferPair(ref, allocator);
+ }
+
+ @Override
+ public TransferPair getTransferPair(BufferAllocator allocator) {
+ return getTransferPair(this.getField().getName(), allocator);
+ }
+
+ @Override
+ public void copyFrom(int fromIndex, int thisIndex, ValueVector from) {
+ getUnderlyingVector()
+ .copyFrom(fromIndex, thisIndex, ((VariantVector) from).getUnderlyingVector());
+ }
+
+ @Override
+ public void copyFromSafe(int fromIndex, int thisIndex, ValueVector from) {
+ getUnderlyingVector()
+ .copyFromSafe(fromIndex, thisIndex, ((VariantVector) from).getUnderlyingVector());
+ }
+
+ @Override
+ public Object getObject(int index) {
+ return getUnderlyingVector().getObject(index);
+ }
Review Comment:
Yes, that was my concern about `parquet-variant`: the "parent" and transitive dependencies. I don't think we should fetch a large part of Parquet in Arrow.
I have a preference to create `arrow-variant` module with clearly defined dependencies.
Thoughts ?
--
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-946: Add Variant extension type support [arrow-java]
Posted by "gszadovszky (via GitHub)" <gi...@apache.org>.
gszadovszky commented on code in PR #947:
URL: https://github.com/apache/arrow-java/pull/947#discussion_r2715795609
##########
vector/src/main/java/org/apache/arrow/vector/extension/VariantVector.java:
##########
@@ -0,0 +1,306 @@
+/*
+ * 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 java.util.List;
+import org.apache.arrow.memory.BufferAllocator;
+import org.apache.arrow.memory.util.hash.ArrowBufHasher;
+import org.apache.arrow.vector.BitVectorHelper;
+import org.apache.arrow.vector.ExtensionTypeVector;
+import org.apache.arrow.vector.FieldVector;
+import org.apache.arrow.vector.ValueVector;
+import org.apache.arrow.vector.VarBinaryVector;
+import org.apache.arrow.vector.complex.AbstractStructVector;
+import org.apache.arrow.vector.complex.StructVector;
+import org.apache.arrow.vector.complex.reader.FieldReader;
+import org.apache.arrow.vector.holders.NullableVariantHolder;
+import org.apache.arrow.vector.holders.VariantHolder;
+import org.apache.arrow.vector.types.pojo.ArrowType;
+import org.apache.arrow.vector.types.pojo.ArrowType.Binary;
+import org.apache.arrow.vector.types.pojo.Field;
+import org.apache.arrow.vector.types.pojo.FieldType;
+import org.apache.arrow.vector.util.CallBack;
+import org.apache.arrow.vector.util.TransferPair;
+
+public class VariantVector extends ExtensionTypeVector<StructVector> {
+
+ public static final String METADATA_VECTOR_NAME = "metadata";
+ public static final String VALUE_VECTOR_NAME = "value";
+
+ private final Field rootField;
+
+ /**
+ * Constructs a new VariantVector with the given name and allocator.
+ *
+ * @param name the name of the vector
+ * @param allocator the buffer allocator for memory management
+ */
+ public VariantVector(String name, BufferAllocator allocator) {
+ super(
+ name,
+ allocator,
+ new StructVector(
+ name,
+ allocator,
+ FieldType.nullable(ArrowType.Struct.INSTANCE),
+ null,
+ AbstractStructVector.ConflictPolicy.CONFLICT_ERROR,
+ false));
+ rootField = createVariantField(name);
+ ((FieldVector) this.getUnderlyingVector())
+ .initializeChildrenFromFields(rootField.getChildren());
+ }
+
+ /**
+ * Creates a new VariantVector with the given name. The Variant Field schema has to be the same
+ * everywhere, otherwise ArrowBuffer loading might fail during serialization/deserialization and
+ * schema mismatches can occur. This includes CompleteType's VARIANT and VARIANT_REQUIRED types.
+ */
+ public static Field createVariantField(String name) {
+ return new Field(
+ name, new FieldType(true, VariantType.INSTANCE, null), createVariantChildFields());
+ }
+
+ /**
+ * Creates the child fields for the VariantVector. Metadata vector will be index 0 and value
+ * vector will be index 1.
+ */
+ public static List<Field> createVariantChildFields() {
+ return List.of(
+ new Field(METADATA_VECTOR_NAME, new FieldType(false, Binary.INSTANCE, null), null),
+ new Field(VALUE_VECTOR_NAME, new FieldType(false, Binary.INSTANCE, null), null));
+ }
+
+ @Override
+ public void initializeChildrenFromFields(List<Field> children) {
+ // No-op, as children are initialized in the constructor
+ }
+
+ @Override
+ public Field getField() {
+ return rootField;
+ }
+
+ public VarBinaryVector getMetadataVector() {
+ return getUnderlyingVector().getChild(METADATA_VECTOR_NAME, VarBinaryVector.class);
+ }
+
+ public VarBinaryVector getValueVector() {
+ return getUnderlyingVector().getChild(VALUE_VECTOR_NAME, VarBinaryVector.class);
+ }
+
+ @Override
+ public TransferPair makeTransferPair(ValueVector target) {
+ return new VariantTransferPair(this, (VariantVector) target);
+ }
+
+ @Override
+ public TransferPair getTransferPair(Field field, BufferAllocator allocator) {
+ return new VariantTransferPair(this, new VariantVector(field.getName(), allocator));
+ }
+
+ @Override
+ public TransferPair getTransferPair(Field field, BufferAllocator allocator, CallBack callBack) {
+ return getTransferPair(field, allocator);
+ }
+
+ @Override
+ public TransferPair getTransferPair(String ref, BufferAllocator allocator) {
+ return new VariantTransferPair(this, new VariantVector(ref, allocator));
+ }
+
+ @Override
+ public TransferPair getTransferPair(String ref, BufferAllocator allocator, CallBack callBack) {
+ return getTransferPair(ref, allocator);
+ }
+
+ @Override
+ public TransferPair getTransferPair(BufferAllocator allocator) {
+ return getTransferPair(this.getField().getName(), allocator);
+ }
+
+ @Override
+ public void copyFrom(int fromIndex, int thisIndex, ValueVector from) {
+ getUnderlyingVector()
+ .copyFrom(fromIndex, thisIndex, ((VariantVector) from).getUnderlyingVector());
+ }
+
+ @Override
+ public void copyFromSafe(int fromIndex, int thisIndex, ValueVector from) {
+ getUnderlyingVector()
+ .copyFromSafe(fromIndex, thisIndex, ((VariantVector) from).getUnderlyingVector());
+ }
+
+ @Override
+ public Object getObject(int index) {
+ return getUnderlyingVector().getObject(index);
+ }
Review Comment:
@jbonofre, I was thinking about only the `arrow-variant` module that was created for the sole purpose of having the variant binary format related code in one place so it is easy to use as a dependency. It contains 10 prod classes all together plus test code. I do agree to wrap this around to extend and avoid exposing its classes in the Arrow API.
I don't think it would be better to duplicate the code instead of using it.
--
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-946: Add Variant extension type support [arrow-java]
Posted by "tmater (via GitHub)" <gi...@apache.org>.
tmater commented on code in PR #947:
URL: https://github.com/apache/arrow-java/pull/947#discussion_r2736610131
##########
arrow-variant/src/main/java/org/apache/arrow/variant/Variant.java:
##########
@@ -0,0 +1,205 @@
+/*
+ * 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.variant;
+
+import java.math.BigDecimal;
+import java.nio.ByteBuffer;
+import java.util.Objects;
+import java.util.UUID;
+import org.apache.arrow.memory.ArrowBuf;
+
+/**
+ * Wrapper around parquet-variant's Variant implementation.
+ *
+ * <p>This wrapper exists to isolate the parquet-variant dependency from Arrow's public API,
+ * allowing the vector module to expose variant functionality without requiring users to depend on
+ * parquet-variant directly. It also ensures that nested variant values (from arrays and objects)
+ * are consistently wrapped.
+ */
+public class Variant {
+
+ private final org.apache.parquet.variant.Variant delegate;
+
+ /** Creates a Variant from raw metadata and value byte arrays. */
+ public Variant(byte[] metadata, byte[] value) {
+ this.delegate = new org.apache.parquet.variant.Variant(value, metadata);
+ }
+
+ /** Creates a Variant by copying data from ArrowBuf instances. */
+ public Variant(
+ ArrowBuf metadataBuffer,
+ int metadataStart,
+ int metadataEnd,
+ ArrowBuf valueBuffer,
+ int valueStart,
+ int valueEnd) {
+ byte[] metadata = new byte[metadataEnd - metadataStart];
+ byte[] value = new byte[valueEnd - valueStart];
+ metadataBuffer.getBytes(metadataStart, metadata);
+ valueBuffer.getBytes(valueStart, value);
+ this.delegate = new org.apache.parquet.variant.Variant(value, metadata);
+ }
+
+ private Variant(org.apache.parquet.variant.Variant delegate) {
+ this.delegate = delegate;
+ }
+
+ public ByteBuffer getValueBuffer() {
+ return delegate.getValueBuffer();
+ }
+
+ public ByteBuffer getMetadataBuffer() {
+ return delegate.getMetadataBuffer();
+ }
+
Review Comment:
This follows how `parquet-variant` defines it, the ByteBuffer is read-only.
I’m happy to change it; I just wanted to call it out.
--
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-946: Add Variant extension type support [arrow-java]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #947:
URL: https://github.com/apache/arrow-java/pull/947#discussion_r2714990619
##########
vector/src/main/java/org/apache/arrow/vector/extension/VariantVector.java:
##########
@@ -0,0 +1,306 @@
+/*
+ * 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 java.util.List;
+import org.apache.arrow.memory.BufferAllocator;
+import org.apache.arrow.memory.util.hash.ArrowBufHasher;
+import org.apache.arrow.vector.BitVectorHelper;
+import org.apache.arrow.vector.ExtensionTypeVector;
+import org.apache.arrow.vector.FieldVector;
+import org.apache.arrow.vector.ValueVector;
+import org.apache.arrow.vector.VarBinaryVector;
+import org.apache.arrow.vector.complex.AbstractStructVector;
+import org.apache.arrow.vector.complex.StructVector;
+import org.apache.arrow.vector.complex.reader.FieldReader;
+import org.apache.arrow.vector.holders.NullableVariantHolder;
+import org.apache.arrow.vector.holders.VariantHolder;
+import org.apache.arrow.vector.types.pojo.ArrowType;
+import org.apache.arrow.vector.types.pojo.ArrowType.Binary;
+import org.apache.arrow.vector.types.pojo.Field;
+import org.apache.arrow.vector.types.pojo.FieldType;
+import org.apache.arrow.vector.util.CallBack;
+import org.apache.arrow.vector.util.TransferPair;
+
+public class VariantVector extends ExtensionTypeVector<StructVector> {
+
+ public static final String METADATA_VECTOR_NAME = "metadata";
+ public static final String VALUE_VECTOR_NAME = "value";
+
+ private final Field rootField;
+
+ /**
+ * Constructs a new VariantVector with the given name and allocator.
+ *
+ * @param name the name of the vector
+ * @param allocator the buffer allocator for memory management
+ */
+ public VariantVector(String name, BufferAllocator allocator) {
+ super(
+ name,
+ allocator,
+ new StructVector(
+ name,
+ allocator,
+ FieldType.nullable(ArrowType.Struct.INSTANCE),
+ null,
+ AbstractStructVector.ConflictPolicy.CONFLICT_ERROR,
+ false));
+ rootField = createVariantField(name);
+ ((FieldVector) this.getUnderlyingVector())
+ .initializeChildrenFromFields(rootField.getChildren());
+ }
+
+ /**
+ * Creates a new VariantVector with the given name. The Variant Field schema has to be the same
+ * everywhere, otherwise ArrowBuffer loading might fail during serialization/deserialization and
+ * schema mismatches can occur. This includes CompleteType's VARIANT and VARIANT_REQUIRED types.
+ */
+ public static Field createVariantField(String name) {
+ return new Field(
+ name, new FieldType(true, VariantType.INSTANCE, null), createVariantChildFields());
+ }
+
+ /**
+ * Creates the child fields for the VariantVector. Metadata vector will be index 0 and value
+ * vector will be index 1.
+ */
+ public static List<Field> createVariantChildFields() {
+ return List.of(
+ new Field(METADATA_VECTOR_NAME, new FieldType(false, Binary.INSTANCE, null), null),
+ new Field(VALUE_VECTOR_NAME, new FieldType(false, Binary.INSTANCE, null), null));
+ }
+
+ @Override
+ public void initializeChildrenFromFields(List<Field> children) {
+ // No-op, as children are initialized in the constructor
+ }
+
+ @Override
+ public Field getField() {
+ return rootField;
+ }
+
+ public VarBinaryVector getMetadataVector() {
+ return getUnderlyingVector().getChild(METADATA_VECTOR_NAME, VarBinaryVector.class);
+ }
+
+ public VarBinaryVector getValueVector() {
+ return getUnderlyingVector().getChild(VALUE_VECTOR_NAME, VarBinaryVector.class);
+ }
+
+ @Override
+ public TransferPair makeTransferPair(ValueVector target) {
+ return new VariantTransferPair(this, (VariantVector) target);
+ }
+
+ @Override
+ public TransferPair getTransferPair(Field field, BufferAllocator allocator) {
+ return new VariantTransferPair(this, new VariantVector(field.getName(), allocator));
+ }
+
+ @Override
+ public TransferPair getTransferPair(Field field, BufferAllocator allocator, CallBack callBack) {
+ return getTransferPair(field, allocator);
+ }
+
+ @Override
+ public TransferPair getTransferPair(String ref, BufferAllocator allocator) {
+ return new VariantTransferPair(this, new VariantVector(ref, allocator));
+ }
+
+ @Override
+ public TransferPair getTransferPair(String ref, BufferAllocator allocator, CallBack callBack) {
+ return getTransferPair(ref, allocator);
+ }
+
+ @Override
+ public TransferPair getTransferPair(BufferAllocator allocator) {
+ return getTransferPair(this.getField().getName(), allocator);
+ }
+
+ @Override
+ public void copyFrom(int fromIndex, int thisIndex, ValueVector from) {
+ getUnderlyingVector()
+ .copyFrom(fromIndex, thisIndex, ((VariantVector) from).getUnderlyingVector());
+ }
+
+ @Override
+ public void copyFromSafe(int fromIndex, int thisIndex, ValueVector from) {
+ getUnderlyingVector()
+ .copyFromSafe(fromIndex, thisIndex, ((VariantVector) from).getUnderlyingVector());
+ }
+
+ @Override
+ public Object getObject(int index) {
+ return getUnderlyingVector().getObject(index);
+ }
Review Comment:
As it stands, this returns the raw struct row? I suppose decoding to a Java object may be what's expected but that may be expensive. Or returning an explicit Variant object that has methods for inspecting/manipulating the data in that row
--
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-946: Add Variant extension type support [arrow-java]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on PR #947:
URL: https://github.com/apache/arrow-java/pull/947#issuecomment-3908494390
@tmater do you mind to rebase ?
--
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-946: Add Variant extension type support [arrow-java]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on PR #947:
URL: https://github.com/apache/arrow-java/pull/947#issuecomment-3811726061
@gszadovszky thanks ! I just ran the CI and fix the PR meta (milestone). I will do a new pass asap.
--
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-946: Add Variant extension type support [arrow-java]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on code in PR #947:
URL: https://github.com/apache/arrow-java/pull/947#discussion_r2715757658
##########
vector/src/main/java/org/apache/arrow/vector/extension/VariantVector.java:
##########
@@ -0,0 +1,306 @@
+/*
+ * 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 java.util.List;
+import org.apache.arrow.memory.BufferAllocator;
+import org.apache.arrow.memory.util.hash.ArrowBufHasher;
+import org.apache.arrow.vector.BitVectorHelper;
+import org.apache.arrow.vector.ExtensionTypeVector;
+import org.apache.arrow.vector.FieldVector;
+import org.apache.arrow.vector.ValueVector;
+import org.apache.arrow.vector.VarBinaryVector;
+import org.apache.arrow.vector.complex.AbstractStructVector;
+import org.apache.arrow.vector.complex.StructVector;
+import org.apache.arrow.vector.complex.reader.FieldReader;
+import org.apache.arrow.vector.holders.NullableVariantHolder;
+import org.apache.arrow.vector.holders.VariantHolder;
+import org.apache.arrow.vector.types.pojo.ArrowType;
+import org.apache.arrow.vector.types.pojo.ArrowType.Binary;
+import org.apache.arrow.vector.types.pojo.Field;
+import org.apache.arrow.vector.types.pojo.FieldType;
+import org.apache.arrow.vector.util.CallBack;
+import org.apache.arrow.vector.util.TransferPair;
+
+public class VariantVector extends ExtensionTypeVector<StructVector> {
+
+ public static final String METADATA_VECTOR_NAME = "metadata";
+ public static final String VALUE_VECTOR_NAME = "value";
+
+ private final Field rootField;
+
+ /**
+ * Constructs a new VariantVector with the given name and allocator.
+ *
+ * @param name the name of the vector
+ * @param allocator the buffer allocator for memory management
+ */
+ public VariantVector(String name, BufferAllocator allocator) {
+ super(
+ name,
+ allocator,
+ new StructVector(
+ name,
+ allocator,
+ FieldType.nullable(ArrowType.Struct.INSTANCE),
+ null,
+ AbstractStructVector.ConflictPolicy.CONFLICT_ERROR,
+ false));
+ rootField = createVariantField(name);
+ ((FieldVector) this.getUnderlyingVector())
+ .initializeChildrenFromFields(rootField.getChildren());
+ }
+
+ /**
+ * Creates a new VariantVector with the given name. The Variant Field schema has to be the same
+ * everywhere, otherwise ArrowBuffer loading might fail during serialization/deserialization and
+ * schema mismatches can occur. This includes CompleteType's VARIANT and VARIANT_REQUIRED types.
+ */
+ public static Field createVariantField(String name) {
+ return new Field(
+ name, new FieldType(true, VariantType.INSTANCE, null), createVariantChildFields());
+ }
+
+ /**
+ * Creates the child fields for the VariantVector. Metadata vector will be index 0 and value
+ * vector will be index 1.
+ */
+ public static List<Field> createVariantChildFields() {
+ return List.of(
+ new Field(METADATA_VECTOR_NAME, new FieldType(false, Binary.INSTANCE, null), null),
+ new Field(VALUE_VECTOR_NAME, new FieldType(false, Binary.INSTANCE, null), null));
+ }
+
+ @Override
+ public void initializeChildrenFromFields(List<Field> children) {
+ // No-op, as children are initialized in the constructor
+ }
+
+ @Override
+ public Field getField() {
+ return rootField;
+ }
+
+ public VarBinaryVector getMetadataVector() {
+ return getUnderlyingVector().getChild(METADATA_VECTOR_NAME, VarBinaryVector.class);
+ }
+
+ public VarBinaryVector getValueVector() {
+ return getUnderlyingVector().getChild(VALUE_VECTOR_NAME, VarBinaryVector.class);
+ }
+
+ @Override
+ public TransferPair makeTransferPair(ValueVector target) {
+ return new VariantTransferPair(this, (VariantVector) target);
+ }
+
+ @Override
+ public TransferPair getTransferPair(Field field, BufferAllocator allocator) {
+ return new VariantTransferPair(this, new VariantVector(field.getName(), allocator));
+ }
+
+ @Override
+ public TransferPair getTransferPair(Field field, BufferAllocator allocator, CallBack callBack) {
+ return getTransferPair(field, allocator);
+ }
+
+ @Override
+ public TransferPair getTransferPair(String ref, BufferAllocator allocator) {
+ return new VariantTransferPair(this, new VariantVector(ref, allocator));
+ }
+
+ @Override
+ public TransferPair getTransferPair(String ref, BufferAllocator allocator, CallBack callBack) {
+ return getTransferPair(ref, allocator);
+ }
+
+ @Override
+ public TransferPair getTransferPair(BufferAllocator allocator) {
+ return getTransferPair(this.getField().getName(), allocator);
+ }
+
+ @Override
+ public void copyFrom(int fromIndex, int thisIndex, ValueVector from) {
+ getUnderlyingVector()
+ .copyFrom(fromIndex, thisIndex, ((VariantVector) from).getUnderlyingVector());
+ }
+
+ @Override
+ public void copyFromSafe(int fromIndex, int thisIndex, ValueVector from) {
+ getUnderlyingVector()
+ .copyFromSafe(fromIndex, thisIndex, ((VariantVector) from).getUnderlyingVector());
+ }
+
+ @Override
+ public Object getObject(int index) {
+ return getUnderlyingVector().getObject(index);
+ }
Review Comment:
Generally speaking, I'm not super fan of introducing a "large" dependency for a single feature. However, for the case of variant, imho, it could be acceptable if we "narrow" it to a specific module (`arrow-variant`), using Parqet classes as "util classes", not exposed to Arrow users.
I would like to check how big is Variant in Parquet, because we can also duplicate the code (it's totally fine if mentioned in the Arrow `LICENSE`).
--
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-946: Add Variant extension type support [arrow-java]
Posted by "gszadovszky (via GitHub)" <gi...@apache.org>.
gszadovszky commented on code in PR #947:
URL: https://github.com/apache/arrow-java/pull/947#discussion_r2713016819
##########
vector/src/main/java/org/apache/arrow/vector/extension/VariantVector.java:
##########
@@ -0,0 +1,306 @@
+/*
+ * 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 java.util.List;
+import org.apache.arrow.memory.BufferAllocator;
+import org.apache.arrow.memory.util.hash.ArrowBufHasher;
+import org.apache.arrow.vector.BitVectorHelper;
+import org.apache.arrow.vector.ExtensionTypeVector;
+import org.apache.arrow.vector.FieldVector;
+import org.apache.arrow.vector.ValueVector;
+import org.apache.arrow.vector.VarBinaryVector;
+import org.apache.arrow.vector.complex.AbstractStructVector;
+import org.apache.arrow.vector.complex.StructVector;
+import org.apache.arrow.vector.complex.reader.FieldReader;
+import org.apache.arrow.vector.holders.NullableVariantHolder;
+import org.apache.arrow.vector.holders.VariantHolder;
+import org.apache.arrow.vector.types.pojo.ArrowType;
+import org.apache.arrow.vector.types.pojo.ArrowType.Binary;
+import org.apache.arrow.vector.types.pojo.Field;
+import org.apache.arrow.vector.types.pojo.FieldType;
+import org.apache.arrow.vector.util.CallBack;
+import org.apache.arrow.vector.util.TransferPair;
+
+public class VariantVector extends ExtensionTypeVector<StructVector> {
+
+ public static final String METADATA_VECTOR_NAME = "metadata";
+ public static final String VALUE_VECTOR_NAME = "value";
+
+ private final Field rootField;
+
+ /**
+ * Constructs a new VariantVector with the given name and allocator.
+ *
+ * @param name the name of the vector
+ * @param allocator the buffer allocator for memory management
+ */
+ public VariantVector(String name, BufferAllocator allocator) {
+ super(
+ name,
+ allocator,
+ new StructVector(
+ name,
+ allocator,
+ FieldType.nullable(ArrowType.Struct.INSTANCE),
+ null,
+ AbstractStructVector.ConflictPolicy.CONFLICT_ERROR,
+ false));
+ rootField = createVariantField(name);
+ ((FieldVector) this.getUnderlyingVector())
+ .initializeChildrenFromFields(rootField.getChildren());
+ }
+
+ /**
+ * Creates a new VariantVector with the given name. The Variant Field schema has to be the same
+ * everywhere, otherwise ArrowBuffer loading might fail during serialization/deserialization and
+ * schema mismatches can occur. This includes CompleteType's VARIANT and VARIANT_REQUIRED types.
+ */
+ public static Field createVariantField(String name) {
+ return new Field(
+ name, new FieldType(true, VariantType.INSTANCE, null), createVariantChildFields());
+ }
+
+ /**
+ * Creates the child fields for the VariantVector. Metadata vector will be index 0 and value
+ * vector will be index 1.
+ */
+ public static List<Field> createVariantChildFields() {
+ return List.of(
+ new Field(METADATA_VECTOR_NAME, new FieldType(false, Binary.INSTANCE, null), null),
+ new Field(VALUE_VECTOR_NAME, new FieldType(false, Binary.INSTANCE, null), null));
+ }
+
+ @Override
+ public void initializeChildrenFromFields(List<Field> children) {
+ // No-op, as children are initialized in the constructor
+ }
+
+ @Override
+ public Field getField() {
+ return rootField;
+ }
+
+ public VarBinaryVector getMetadataVector() {
+ return getUnderlyingVector().getChild(METADATA_VECTOR_NAME, VarBinaryVector.class);
+ }
+
+ public VarBinaryVector getValueVector() {
+ return getUnderlyingVector().getChild(VALUE_VECTOR_NAME, VarBinaryVector.class);
+ }
+
+ @Override
+ public TransferPair makeTransferPair(ValueVector target) {
+ return new VariantTransferPair(this, (VariantVector) target);
+ }
+
+ @Override
+ public TransferPair getTransferPair(Field field, BufferAllocator allocator) {
+ return new VariantTransferPair(this, new VariantVector(field.getName(), allocator));
+ }
+
+ @Override
+ public TransferPair getTransferPair(Field field, BufferAllocator allocator, CallBack callBack) {
+ return getTransferPair(field, allocator);
+ }
+
+ @Override
+ public TransferPair getTransferPair(String ref, BufferAllocator allocator) {
+ return new VariantTransferPair(this, new VariantVector(ref, allocator));
+ }
+
+ @Override
+ public TransferPair getTransferPair(String ref, BufferAllocator allocator, CallBack callBack) {
+ return getTransferPair(ref, allocator);
+ }
+
+ @Override
+ public TransferPair getTransferPair(BufferAllocator allocator) {
+ return getTransferPair(this.getField().getName(), allocator);
+ }
+
+ @Override
+ public void copyFrom(int fromIndex, int thisIndex, ValueVector from) {
+ getUnderlyingVector()
+ .copyFrom(fromIndex, thisIndex, ((VariantVector) from).getUnderlyingVector());
+ }
+
+ @Override
+ public void copyFromSafe(int fromIndex, int thisIndex, ValueVector from) {
+ getUnderlyingVector()
+ .copyFromSafe(fromIndex, thisIndex, ((VariantVector) from).getUnderlyingVector());
+ }
+
+ @Override
+ public Object getObject(int index) {
+ return getUnderlyingVector().getObject(index);
+ }
Review Comment:
I'm not sure if it is useful this way. I don't have a strong opinion though what should it return instead. @lidavidm, do you have an idea?
--
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-946: Add Variant extension type support [arrow-java]
Posted by "gszadovszky (via GitHub)" <gi...@apache.org>.
gszadovszky commented on code in PR #947:
URL: https://github.com/apache/arrow-java/pull/947#discussion_r2715709051
##########
vector/src/main/java/org/apache/arrow/vector/extension/VariantVector.java:
##########
@@ -0,0 +1,306 @@
+/*
+ * 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 java.util.List;
+import org.apache.arrow.memory.BufferAllocator;
+import org.apache.arrow.memory.util.hash.ArrowBufHasher;
+import org.apache.arrow.vector.BitVectorHelper;
+import org.apache.arrow.vector.ExtensionTypeVector;
+import org.apache.arrow.vector.FieldVector;
+import org.apache.arrow.vector.ValueVector;
+import org.apache.arrow.vector.VarBinaryVector;
+import org.apache.arrow.vector.complex.AbstractStructVector;
+import org.apache.arrow.vector.complex.StructVector;
+import org.apache.arrow.vector.complex.reader.FieldReader;
+import org.apache.arrow.vector.holders.NullableVariantHolder;
+import org.apache.arrow.vector.holders.VariantHolder;
+import org.apache.arrow.vector.types.pojo.ArrowType;
+import org.apache.arrow.vector.types.pojo.ArrowType.Binary;
+import org.apache.arrow.vector.types.pojo.Field;
+import org.apache.arrow.vector.types.pojo.FieldType;
+import org.apache.arrow.vector.util.CallBack;
+import org.apache.arrow.vector.util.TransferPair;
+
+public class VariantVector extends ExtensionTypeVector<StructVector> {
+
+ public static final String METADATA_VECTOR_NAME = "metadata";
+ public static final String VALUE_VECTOR_NAME = "value";
+
+ private final Field rootField;
+
+ /**
+ * Constructs a new VariantVector with the given name and allocator.
+ *
+ * @param name the name of the vector
+ * @param allocator the buffer allocator for memory management
+ */
+ public VariantVector(String name, BufferAllocator allocator) {
+ super(
+ name,
+ allocator,
+ new StructVector(
+ name,
+ allocator,
+ FieldType.nullable(ArrowType.Struct.INSTANCE),
+ null,
+ AbstractStructVector.ConflictPolicy.CONFLICT_ERROR,
+ false));
+ rootField = createVariantField(name);
+ ((FieldVector) this.getUnderlyingVector())
+ .initializeChildrenFromFields(rootField.getChildren());
+ }
+
+ /**
+ * Creates a new VariantVector with the given name. The Variant Field schema has to be the same
+ * everywhere, otherwise ArrowBuffer loading might fail during serialization/deserialization and
+ * schema mismatches can occur. This includes CompleteType's VARIANT and VARIANT_REQUIRED types.
+ */
+ public static Field createVariantField(String name) {
+ return new Field(
+ name, new FieldType(true, VariantType.INSTANCE, null), createVariantChildFields());
+ }
+
+ /**
+ * Creates the child fields for the VariantVector. Metadata vector will be index 0 and value
+ * vector will be index 1.
+ */
+ public static List<Field> createVariantChildFields() {
+ return List.of(
+ new Field(METADATA_VECTOR_NAME, new FieldType(false, Binary.INSTANCE, null), null),
+ new Field(VALUE_VECTOR_NAME, new FieldType(false, Binary.INSTANCE, null), null));
+ }
+
+ @Override
+ public void initializeChildrenFromFields(List<Field> children) {
+ // No-op, as children are initialized in the constructor
+ }
+
+ @Override
+ public Field getField() {
+ return rootField;
+ }
+
+ public VarBinaryVector getMetadataVector() {
+ return getUnderlyingVector().getChild(METADATA_VECTOR_NAME, VarBinaryVector.class);
+ }
+
+ public VarBinaryVector getValueVector() {
+ return getUnderlyingVector().getChild(VALUE_VECTOR_NAME, VarBinaryVector.class);
+ }
+
+ @Override
+ public TransferPair makeTransferPair(ValueVector target) {
+ return new VariantTransferPair(this, (VariantVector) target);
+ }
+
+ @Override
+ public TransferPair getTransferPair(Field field, BufferAllocator allocator) {
+ return new VariantTransferPair(this, new VariantVector(field.getName(), allocator));
+ }
+
+ @Override
+ public TransferPair getTransferPair(Field field, BufferAllocator allocator, CallBack callBack) {
+ return getTransferPair(field, allocator);
+ }
+
+ @Override
+ public TransferPair getTransferPair(String ref, BufferAllocator allocator) {
+ return new VariantTransferPair(this, new VariantVector(ref, allocator));
+ }
+
+ @Override
+ public TransferPair getTransferPair(String ref, BufferAllocator allocator, CallBack callBack) {
+ return getTransferPair(ref, allocator);
+ }
+
+ @Override
+ public TransferPair getTransferPair(BufferAllocator allocator) {
+ return getTransferPair(this.getField().getName(), allocator);
+ }
+
+ @Override
+ public void copyFrom(int fromIndex, int thisIndex, ValueVector from) {
+ getUnderlyingVector()
+ .copyFrom(fromIndex, thisIndex, ((VariantVector) from).getUnderlyingVector());
+ }
+
+ @Override
+ public void copyFromSafe(int fromIndex, int thisIndex, ValueVector from) {
+ getUnderlyingVector()
+ .copyFromSafe(fromIndex, thisIndex, ((VariantVector) from).getUnderlyingVector());
+ }
+
+ @Override
+ public Object getObject(int index) {
+ return getUnderlyingVector().getObject(index);
+ }
Review Comment:
To be sure I understand you correctly, @lidavidm, do you agree with the following?
Add the new parquet-java dependency and use its variant implementation as a bases of a new Arrow Variant class to be returned by `VariantVector.getObject`. This class can return the corresponding java objects for the related primitives according to the java objects returned by their typed vectors. No parquet-java classes shall face the public API of Arrow.
--
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-946: Add Variant extension type support [arrow-java]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #947:
URL: https://github.com/apache/arrow-java/pull/947#discussion_r2715679855
##########
vector/src/main/java/org/apache/arrow/vector/extension/VariantVector.java:
##########
@@ -0,0 +1,306 @@
+/*
+ * 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 java.util.List;
+import org.apache.arrow.memory.BufferAllocator;
+import org.apache.arrow.memory.util.hash.ArrowBufHasher;
+import org.apache.arrow.vector.BitVectorHelper;
+import org.apache.arrow.vector.ExtensionTypeVector;
+import org.apache.arrow.vector.FieldVector;
+import org.apache.arrow.vector.ValueVector;
+import org.apache.arrow.vector.VarBinaryVector;
+import org.apache.arrow.vector.complex.AbstractStructVector;
+import org.apache.arrow.vector.complex.StructVector;
+import org.apache.arrow.vector.complex.reader.FieldReader;
+import org.apache.arrow.vector.holders.NullableVariantHolder;
+import org.apache.arrow.vector.holders.VariantHolder;
+import org.apache.arrow.vector.types.pojo.ArrowType;
+import org.apache.arrow.vector.types.pojo.ArrowType.Binary;
+import org.apache.arrow.vector.types.pojo.Field;
+import org.apache.arrow.vector.types.pojo.FieldType;
+import org.apache.arrow.vector.util.CallBack;
+import org.apache.arrow.vector.util.TransferPair;
+
+public class VariantVector extends ExtensionTypeVector<StructVector> {
+
+ public static final String METADATA_VECTOR_NAME = "metadata";
+ public static final String VALUE_VECTOR_NAME = "value";
+
+ private final Field rootField;
+
+ /**
+ * Constructs a new VariantVector with the given name and allocator.
+ *
+ * @param name the name of the vector
+ * @param allocator the buffer allocator for memory management
+ */
+ public VariantVector(String name, BufferAllocator allocator) {
+ super(
+ name,
+ allocator,
+ new StructVector(
+ name,
+ allocator,
+ FieldType.nullable(ArrowType.Struct.INSTANCE),
+ null,
+ AbstractStructVector.ConflictPolicy.CONFLICT_ERROR,
+ false));
+ rootField = createVariantField(name);
+ ((FieldVector) this.getUnderlyingVector())
+ .initializeChildrenFromFields(rootField.getChildren());
+ }
+
+ /**
+ * Creates a new VariantVector with the given name. The Variant Field schema has to be the same
+ * everywhere, otherwise ArrowBuffer loading might fail during serialization/deserialization and
+ * schema mismatches can occur. This includes CompleteType's VARIANT and VARIANT_REQUIRED types.
+ */
+ public static Field createVariantField(String name) {
+ return new Field(
+ name, new FieldType(true, VariantType.INSTANCE, null), createVariantChildFields());
+ }
+
+ /**
+ * Creates the child fields for the VariantVector. Metadata vector will be index 0 and value
+ * vector will be index 1.
+ */
+ public static List<Field> createVariantChildFields() {
+ return List.of(
+ new Field(METADATA_VECTOR_NAME, new FieldType(false, Binary.INSTANCE, null), null),
+ new Field(VALUE_VECTOR_NAME, new FieldType(false, Binary.INSTANCE, null), null));
+ }
+
+ @Override
+ public void initializeChildrenFromFields(List<Field> children) {
+ // No-op, as children are initialized in the constructor
+ }
+
+ @Override
+ public Field getField() {
+ return rootField;
+ }
+
+ public VarBinaryVector getMetadataVector() {
+ return getUnderlyingVector().getChild(METADATA_VECTOR_NAME, VarBinaryVector.class);
+ }
+
+ public VarBinaryVector getValueVector() {
+ return getUnderlyingVector().getChild(VALUE_VECTOR_NAME, VarBinaryVector.class);
+ }
+
+ @Override
+ public TransferPair makeTransferPair(ValueVector target) {
+ return new VariantTransferPair(this, (VariantVector) target);
+ }
+
+ @Override
+ public TransferPair getTransferPair(Field field, BufferAllocator allocator) {
+ return new VariantTransferPair(this, new VariantVector(field.getName(), allocator));
+ }
+
+ @Override
+ public TransferPair getTransferPair(Field field, BufferAllocator allocator, CallBack callBack) {
+ return getTransferPair(field, allocator);
+ }
+
+ @Override
+ public TransferPair getTransferPair(String ref, BufferAllocator allocator) {
+ return new VariantTransferPair(this, new VariantVector(ref, allocator));
+ }
+
+ @Override
+ public TransferPair getTransferPair(String ref, BufferAllocator allocator, CallBack callBack) {
+ return getTransferPair(ref, allocator);
+ }
+
+ @Override
+ public TransferPair getTransferPair(BufferAllocator allocator) {
+ return getTransferPair(this.getField().getName(), allocator);
+ }
+
+ @Override
+ public void copyFrom(int fromIndex, int thisIndex, ValueVector from) {
+ getUnderlyingVector()
+ .copyFrom(fromIndex, thisIndex, ((VariantVector) from).getUnderlyingVector());
+ }
+
+ @Override
+ public void copyFromSafe(int fromIndex, int thisIndex, ValueVector from) {
+ getUnderlyingVector()
+ .copyFromSafe(fromIndex, thisIndex, ((VariantVector) from).getUnderlyingVector());
+ }
+
+ @Override
+ public Object getObject(int index) {
+ return getUnderlyingVector().getObject(index);
+ }
Review Comment:
I think if we go this route, I'd rather have an Arrow Variant to avoid having to break APIs down the line if we decide to extend functionality
--
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-946: Add Variant extension type support [arrow-java]
Posted by "tmater (via GitHub)" <gi...@apache.org>.
tmater commented on code in PR #947:
URL: https://github.com/apache/arrow-java/pull/947#discussion_r2736596193
##########
vector/src/test/java/org/apache/arrow/vector/types/pojo/TestExtensionType.java:
##########
Review Comment:
Created separate classes, but moved to the new module.
--
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-946: Add Variant extension type support [arrow-java]
Posted by "tmater (via GitHub)" <gi...@apache.org>.
tmater commented on PR #947:
URL: https://github.com/apache/arrow-java/pull/947#issuecomment-3718858887
> We're about to overhaul the API for implementing extension writers so you may want to hold off
Thanks @lidavidm, I will wait for that, then rebase. You are talking about the `ExtensionTypeWriterFactory` related changes, right?
--
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-946: Add Variant extension type support [arrow-java]
Posted by "gszadovszky (via GitHub)" <gi...@apache.org>.
gszadovszky commented on code in PR #947:
URL: https://github.com/apache/arrow-java/pull/947#discussion_r2736896951
##########
arrow-variant/src/main/java/org/apache/arrow/variant/Variant.java:
##########
@@ -0,0 +1,205 @@
+/*
+ * 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.variant;
+
+import java.math.BigDecimal;
+import java.nio.ByteBuffer;
+import java.util.Objects;
+import java.util.UUID;
+import org.apache.arrow.memory.ArrowBuf;
+
+/**
+ * Wrapper around parquet-variant's Variant implementation.
+ *
+ * <p>This wrapper exists to isolate the parquet-variant dependency from Arrow's public API,
+ * allowing the vector module to expose variant functionality without requiring users to depend on
+ * parquet-variant directly. It also ensures that nested variant values (from arrays and objects)
+ * are consistently wrapped.
+ */
+public class Variant {
+
+ private final org.apache.parquet.variant.Variant delegate;
+
+ /** Creates a Variant from raw metadata and value byte arrays. */
+ public Variant(byte[] metadata, byte[] value) {
+ this.delegate = new org.apache.parquet.variant.Variant(value, metadata);
+ }
+
+ /** Creates a Variant by copying data from ArrowBuf instances. */
+ public Variant(
+ ArrowBuf metadataBuffer,
+ int metadataStart,
+ int metadataEnd,
+ ArrowBuf valueBuffer,
+ int valueStart,
+ int valueEnd) {
+ byte[] metadata = new byte[metadataEnd - metadataStart];
+ byte[] value = new byte[valueEnd - valueStart];
+ metadataBuffer.getBytes(metadataStart, metadata);
+ valueBuffer.getBytes(valueStart, value);
+ this.delegate = new org.apache.parquet.variant.Variant(value, metadata);
+ }
+
+ private Variant(org.apache.parquet.variant.Variant delegate) {
+ this.delegate = delegate;
+ }
+
+ public ByteBuffer getValueBuffer() {
+ return delegate.getValueBuffer();
+ }
+
+ public ByteBuffer getMetadataBuffer() {
+ return delegate.getMetadataBuffer();
+ }
+
Review Comment:
I'm not sure it is useful for the users, but do not have a strong opinion to remove it.
--
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-946: Add Variant extension type support [arrow-java]
Posted by "tmater (via GitHub)" <gi...@apache.org>.
tmater commented on PR #947:
URL: https://github.com/apache/arrow-java/pull/947#issuecomment-3847997212
Hi folks! Just a gentle ping on this PR to see if there are any updates. Happy to help with anything or make changes if needed — thanks so much for taking a look!
--
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-946: Add Variant extension type support [arrow-java]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on code in PR #947:
URL: https://github.com/apache/arrow-java/pull/947#discussion_r2715873697
##########
vector/src/main/java/org/apache/arrow/vector/extension/VariantVector.java:
##########
@@ -0,0 +1,306 @@
+/*
+ * 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 java.util.List;
+import org.apache.arrow.memory.BufferAllocator;
+import org.apache.arrow.memory.util.hash.ArrowBufHasher;
+import org.apache.arrow.vector.BitVectorHelper;
+import org.apache.arrow.vector.ExtensionTypeVector;
+import org.apache.arrow.vector.FieldVector;
+import org.apache.arrow.vector.ValueVector;
+import org.apache.arrow.vector.VarBinaryVector;
+import org.apache.arrow.vector.complex.AbstractStructVector;
+import org.apache.arrow.vector.complex.StructVector;
+import org.apache.arrow.vector.complex.reader.FieldReader;
+import org.apache.arrow.vector.holders.NullableVariantHolder;
+import org.apache.arrow.vector.holders.VariantHolder;
+import org.apache.arrow.vector.types.pojo.ArrowType;
+import org.apache.arrow.vector.types.pojo.ArrowType.Binary;
+import org.apache.arrow.vector.types.pojo.Field;
+import org.apache.arrow.vector.types.pojo.FieldType;
+import org.apache.arrow.vector.util.CallBack;
+import org.apache.arrow.vector.util.TransferPair;
+
+public class VariantVector extends ExtensionTypeVector<StructVector> {
+
+ public static final String METADATA_VECTOR_NAME = "metadata";
+ public static final String VALUE_VECTOR_NAME = "value";
+
+ private final Field rootField;
+
+ /**
+ * Constructs a new VariantVector with the given name and allocator.
+ *
+ * @param name the name of the vector
+ * @param allocator the buffer allocator for memory management
+ */
+ public VariantVector(String name, BufferAllocator allocator) {
+ super(
+ name,
+ allocator,
+ new StructVector(
+ name,
+ allocator,
+ FieldType.nullable(ArrowType.Struct.INSTANCE),
+ null,
+ AbstractStructVector.ConflictPolicy.CONFLICT_ERROR,
+ false));
+ rootField = createVariantField(name);
+ ((FieldVector) this.getUnderlyingVector())
+ .initializeChildrenFromFields(rootField.getChildren());
+ }
+
+ /**
+ * Creates a new VariantVector with the given name. The Variant Field schema has to be the same
+ * everywhere, otherwise ArrowBuffer loading might fail during serialization/deserialization and
+ * schema mismatches can occur. This includes CompleteType's VARIANT and VARIANT_REQUIRED types.
+ */
+ public static Field createVariantField(String name) {
+ return new Field(
+ name, new FieldType(true, VariantType.INSTANCE, null), createVariantChildFields());
+ }
+
+ /**
+ * Creates the child fields for the VariantVector. Metadata vector will be index 0 and value
+ * vector will be index 1.
+ */
+ public static List<Field> createVariantChildFields() {
+ return List.of(
+ new Field(METADATA_VECTOR_NAME, new FieldType(false, Binary.INSTANCE, null), null),
+ new Field(VALUE_VECTOR_NAME, new FieldType(false, Binary.INSTANCE, null), null));
+ }
+
+ @Override
+ public void initializeChildrenFromFields(List<Field> children) {
+ // No-op, as children are initialized in the constructor
+ }
+
+ @Override
+ public Field getField() {
+ return rootField;
+ }
+
+ public VarBinaryVector getMetadataVector() {
+ return getUnderlyingVector().getChild(METADATA_VECTOR_NAME, VarBinaryVector.class);
+ }
+
+ public VarBinaryVector getValueVector() {
+ return getUnderlyingVector().getChild(VALUE_VECTOR_NAME, VarBinaryVector.class);
+ }
+
+ @Override
+ public TransferPair makeTransferPair(ValueVector target) {
+ return new VariantTransferPair(this, (VariantVector) target);
+ }
+
+ @Override
+ public TransferPair getTransferPair(Field field, BufferAllocator allocator) {
+ return new VariantTransferPair(this, new VariantVector(field.getName(), allocator));
+ }
+
+ @Override
+ public TransferPair getTransferPair(Field field, BufferAllocator allocator, CallBack callBack) {
+ return getTransferPair(field, allocator);
+ }
+
+ @Override
+ public TransferPair getTransferPair(String ref, BufferAllocator allocator) {
+ return new VariantTransferPair(this, new VariantVector(ref, allocator));
+ }
+
+ @Override
+ public TransferPair getTransferPair(String ref, BufferAllocator allocator, CallBack callBack) {
+ return getTransferPair(ref, allocator);
+ }
+
+ @Override
+ public TransferPair getTransferPair(BufferAllocator allocator) {
+ return getTransferPair(this.getField().getName(), allocator);
+ }
+
+ @Override
+ public void copyFrom(int fromIndex, int thisIndex, ValueVector from) {
+ getUnderlyingVector()
+ .copyFrom(fromIndex, thisIndex, ((VariantVector) from).getUnderlyingVector());
+ }
+
+ @Override
+ public void copyFromSafe(int fromIndex, int thisIndex, ValueVector from) {
+ getUnderlyingVector()
+ .copyFromSafe(fromIndex, thisIndex, ((VariantVector) from).getUnderlyingVector());
+ }
+
+ @Override
+ public Object getObject(int index) {
+ return getUnderlyingVector().getObject(index);
+ }
Review Comment:
I agree with `arrow-parquet-variant` approach. It cleanly isolated the dependency (to avoid any side effect in other parts of Arrow).
--
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-946: Add Variant extension type support [arrow-java]
Posted by "tmater (via GitHub)" <gi...@apache.org>.
tmater commented on code in PR #947:
URL: https://github.com/apache/arrow-java/pull/947#discussion_r2735589710
##########
vector/src/main/java/org/apache/arrow/vector/extension/VariantVector.java:
##########
@@ -0,0 +1,306 @@
+/*
+ * 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 java.util.List;
+import org.apache.arrow.memory.BufferAllocator;
+import org.apache.arrow.memory.util.hash.ArrowBufHasher;
+import org.apache.arrow.vector.BitVectorHelper;
+import org.apache.arrow.vector.ExtensionTypeVector;
+import org.apache.arrow.vector.FieldVector;
+import org.apache.arrow.vector.ValueVector;
+import org.apache.arrow.vector.VarBinaryVector;
+import org.apache.arrow.vector.complex.AbstractStructVector;
+import org.apache.arrow.vector.complex.StructVector;
+import org.apache.arrow.vector.complex.reader.FieldReader;
+import org.apache.arrow.vector.holders.NullableVariantHolder;
+import org.apache.arrow.vector.holders.VariantHolder;
+import org.apache.arrow.vector.types.pojo.ArrowType;
+import org.apache.arrow.vector.types.pojo.ArrowType.Binary;
+import org.apache.arrow.vector.types.pojo.Field;
+import org.apache.arrow.vector.types.pojo.FieldType;
+import org.apache.arrow.vector.util.CallBack;
+import org.apache.arrow.vector.util.TransferPair;
+
+public class VariantVector extends ExtensionTypeVector<StructVector> {
+
+ public static final String METADATA_VECTOR_NAME = "metadata";
+ public static final String VALUE_VECTOR_NAME = "value";
+
+ private final Field rootField;
+
+ /**
+ * Constructs a new VariantVector with the given name and allocator.
+ *
+ * @param name the name of the vector
+ * @param allocator the buffer allocator for memory management
+ */
+ public VariantVector(String name, BufferAllocator allocator) {
+ super(
+ name,
+ allocator,
+ new StructVector(
+ name,
+ allocator,
+ FieldType.nullable(ArrowType.Struct.INSTANCE),
+ null,
+ AbstractStructVector.ConflictPolicy.CONFLICT_ERROR,
+ false));
+ rootField = createVariantField(name);
+ ((FieldVector) this.getUnderlyingVector())
+ .initializeChildrenFromFields(rootField.getChildren());
+ }
+
+ /**
+ * Creates a new VariantVector with the given name. The Variant Field schema has to be the same
+ * everywhere, otherwise ArrowBuffer loading might fail during serialization/deserialization and
+ * schema mismatches can occur. This includes CompleteType's VARIANT and VARIANT_REQUIRED types.
+ */
+ public static Field createVariantField(String name) {
+ return new Field(
+ name, new FieldType(true, VariantType.INSTANCE, null), createVariantChildFields());
+ }
+
+ /**
+ * Creates the child fields for the VariantVector. Metadata vector will be index 0 and value
+ * vector will be index 1.
+ */
+ public static List<Field> createVariantChildFields() {
+ return List.of(
+ new Field(METADATA_VECTOR_NAME, new FieldType(false, Binary.INSTANCE, null), null),
+ new Field(VALUE_VECTOR_NAME, new FieldType(false, Binary.INSTANCE, null), null));
+ }
+
+ @Override
+ public void initializeChildrenFromFields(List<Field> children) {
+ // No-op, as children are initialized in the constructor
+ }
+
+ @Override
+ public Field getField() {
+ return rootField;
+ }
+
+ public VarBinaryVector getMetadataVector() {
+ return getUnderlyingVector().getChild(METADATA_VECTOR_NAME, VarBinaryVector.class);
+ }
+
+ public VarBinaryVector getValueVector() {
+ return getUnderlyingVector().getChild(VALUE_VECTOR_NAME, VarBinaryVector.class);
+ }
+
+ @Override
+ public TransferPair makeTransferPair(ValueVector target) {
+ return new VariantTransferPair(this, (VariantVector) target);
+ }
+
+ @Override
+ public TransferPair getTransferPair(Field field, BufferAllocator allocator) {
+ return new VariantTransferPair(this, new VariantVector(field.getName(), allocator));
+ }
+
+ @Override
+ public TransferPair getTransferPair(Field field, BufferAllocator allocator, CallBack callBack) {
+ return getTransferPair(field, allocator);
+ }
+
+ @Override
+ public TransferPair getTransferPair(String ref, BufferAllocator allocator) {
+ return new VariantTransferPair(this, new VariantVector(ref, allocator));
+ }
+
+ @Override
+ public TransferPair getTransferPair(String ref, BufferAllocator allocator, CallBack callBack) {
+ return getTransferPair(ref, allocator);
+ }
+
+ @Override
+ public TransferPair getTransferPair(BufferAllocator allocator) {
+ return getTransferPair(this.getField().getName(), allocator);
+ }
+
+ @Override
+ public void copyFrom(int fromIndex, int thisIndex, ValueVector from) {
+ getUnderlyingVector()
+ .copyFrom(fromIndex, thisIndex, ((VariantVector) from).getUnderlyingVector());
+ }
+
+ @Override
+ public void copyFromSafe(int fromIndex, int thisIndex, ValueVector from) {
+ getUnderlyingVector()
+ .copyFromSafe(fromIndex, thisIndex, ((VariantVector) from).getUnderlyingVector());
+ }
+
+ @Override
+ public Object getObject(int index) {
+ return getUnderlyingVector().getObject(index);
+ }
Review Comment:
Thanks for the discussion! I've updated the PR to introduce a separate `arrow-variant` module that depends on `parquet-variant` for the binary format handling. The `Variant` class wraps the Parquet implementation and is returned by VariantVector.getObject, keeping Parquet classes out of the public Arrow API.
Let me know what you think!
##########
vector/src/main/java/org/apache/arrow/vector/extension/VariantVector.java:
##########
@@ -0,0 +1,306 @@
+/*
+ * 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 java.util.List;
+import org.apache.arrow.memory.BufferAllocator;
+import org.apache.arrow.memory.util.hash.ArrowBufHasher;
+import org.apache.arrow.vector.BitVectorHelper;
+import org.apache.arrow.vector.ExtensionTypeVector;
+import org.apache.arrow.vector.FieldVector;
+import org.apache.arrow.vector.ValueVector;
+import org.apache.arrow.vector.VarBinaryVector;
+import org.apache.arrow.vector.complex.AbstractStructVector;
+import org.apache.arrow.vector.complex.StructVector;
+import org.apache.arrow.vector.complex.reader.FieldReader;
+import org.apache.arrow.vector.holders.NullableVariantHolder;
+import org.apache.arrow.vector.holders.VariantHolder;
+import org.apache.arrow.vector.types.pojo.ArrowType;
+import org.apache.arrow.vector.types.pojo.ArrowType.Binary;
+import org.apache.arrow.vector.types.pojo.Field;
+import org.apache.arrow.vector.types.pojo.FieldType;
+import org.apache.arrow.vector.util.CallBack;
+import org.apache.arrow.vector.util.TransferPair;
+
+public class VariantVector extends ExtensionTypeVector<StructVector> {
+
+ public static final String METADATA_VECTOR_NAME = "metadata";
+ public static final String VALUE_VECTOR_NAME = "value";
+
+ private final Field rootField;
+
+ /**
+ * Constructs a new VariantVector with the given name and allocator.
+ *
+ * @param name the name of the vector
+ * @param allocator the buffer allocator for memory management
+ */
+ public VariantVector(String name, BufferAllocator allocator) {
+ super(
+ name,
+ allocator,
+ new StructVector(
+ name,
+ allocator,
+ FieldType.nullable(ArrowType.Struct.INSTANCE),
+ null,
+ AbstractStructVector.ConflictPolicy.CONFLICT_ERROR,
+ false));
+ rootField = createVariantField(name);
+ ((FieldVector) this.getUnderlyingVector())
+ .initializeChildrenFromFields(rootField.getChildren());
+ }
+
+ /**
+ * Creates a new VariantVector with the given name. The Variant Field schema has to be the same
+ * everywhere, otherwise ArrowBuffer loading might fail during serialization/deserialization and
+ * schema mismatches can occur. This includes CompleteType's VARIANT and VARIANT_REQUIRED types.
+ */
+ public static Field createVariantField(String name) {
+ return new Field(
+ name, new FieldType(true, VariantType.INSTANCE, null), createVariantChildFields());
+ }
+
+ /**
+ * Creates the child fields for the VariantVector. Metadata vector will be index 0 and value
+ * vector will be index 1.
+ */
+ public static List<Field> createVariantChildFields() {
+ return List.of(
+ new Field(METADATA_VECTOR_NAME, new FieldType(false, Binary.INSTANCE, null), null),
+ new Field(VALUE_VECTOR_NAME, new FieldType(false, Binary.INSTANCE, null), null));
+ }
+
+ @Override
+ public void initializeChildrenFromFields(List<Field> children) {
+ // No-op, as children are initialized in the constructor
+ }
+
+ @Override
+ public Field getField() {
+ return rootField;
+ }
+
+ public VarBinaryVector getMetadataVector() {
+ return getUnderlyingVector().getChild(METADATA_VECTOR_NAME, VarBinaryVector.class);
+ }
+
+ public VarBinaryVector getValueVector() {
+ return getUnderlyingVector().getChild(VALUE_VECTOR_NAME, VarBinaryVector.class);
+ }
+
+ @Override
+ public TransferPair makeTransferPair(ValueVector target) {
+ return new VariantTransferPair(this, (VariantVector) target);
+ }
+
+ @Override
+ public TransferPair getTransferPair(Field field, BufferAllocator allocator) {
+ return new VariantTransferPair(this, new VariantVector(field.getName(), allocator));
+ }
+
+ @Override
+ public TransferPair getTransferPair(Field field, BufferAllocator allocator, CallBack callBack) {
+ return getTransferPair(field, allocator);
+ }
+
+ @Override
+ public TransferPair getTransferPair(String ref, BufferAllocator allocator) {
+ return new VariantTransferPair(this, new VariantVector(ref, allocator));
+ }
+
+ @Override
+ public TransferPair getTransferPair(String ref, BufferAllocator allocator, CallBack callBack) {
+ return getTransferPair(ref, allocator);
+ }
+
+ @Override
+ public TransferPair getTransferPair(BufferAllocator allocator) {
+ return getTransferPair(this.getField().getName(), allocator);
+ }
+
+ @Override
+ public void copyFrom(int fromIndex, int thisIndex, ValueVector from) {
+ getUnderlyingVector()
+ .copyFrom(fromIndex, thisIndex, ((VariantVector) from).getUnderlyingVector());
+ }
+
+ @Override
+ public void copyFromSafe(int fromIndex, int thisIndex, ValueVector from) {
+ getUnderlyingVector()
+ .copyFromSafe(fromIndex, thisIndex, ((VariantVector) from).getUnderlyingVector());
+ }
+
+ @Override
+ public Object getObject(int index) {
+ return getUnderlyingVector().getObject(index);
+ }
Review Comment:
Thanks for the discussion! I've updated the PR to introduce a separate `arrow-variant` module that depends on `parquet-variant` for the binary format handling. The `Variant` class wraps the Parquet implementation and is returned by `VariantVector.getObject`, keeping Parquet classes out of the public Arrow API.
Let me know what you think!
--
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-946: Add Variant extension type support [arrow-java]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #947:
URL: https://github.com/apache/arrow-java/pull/947#issuecomment-3717988227
<!-- 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-946: Add Variant extension type support [arrow-java]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #947:
URL: https://github.com/apache/arrow-java/pull/947#issuecomment-3718392489
We're about to overhaul the API for implementing extension writers so you may want to hold off
--
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-946: Add Variant extension type support [arrow-java]
Posted by "gszadovszky (via GitHub)" <gi...@apache.org>.
gszadovszky commented on code in PR #947:
URL: https://github.com/apache/arrow-java/pull/947#discussion_r2715795609
##########
vector/src/main/java/org/apache/arrow/vector/extension/VariantVector.java:
##########
@@ -0,0 +1,306 @@
+/*
+ * 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 java.util.List;
+import org.apache.arrow.memory.BufferAllocator;
+import org.apache.arrow.memory.util.hash.ArrowBufHasher;
+import org.apache.arrow.vector.BitVectorHelper;
+import org.apache.arrow.vector.ExtensionTypeVector;
+import org.apache.arrow.vector.FieldVector;
+import org.apache.arrow.vector.ValueVector;
+import org.apache.arrow.vector.VarBinaryVector;
+import org.apache.arrow.vector.complex.AbstractStructVector;
+import org.apache.arrow.vector.complex.StructVector;
+import org.apache.arrow.vector.complex.reader.FieldReader;
+import org.apache.arrow.vector.holders.NullableVariantHolder;
+import org.apache.arrow.vector.holders.VariantHolder;
+import org.apache.arrow.vector.types.pojo.ArrowType;
+import org.apache.arrow.vector.types.pojo.ArrowType.Binary;
+import org.apache.arrow.vector.types.pojo.Field;
+import org.apache.arrow.vector.types.pojo.FieldType;
+import org.apache.arrow.vector.util.CallBack;
+import org.apache.arrow.vector.util.TransferPair;
+
+public class VariantVector extends ExtensionTypeVector<StructVector> {
+
+ public static final String METADATA_VECTOR_NAME = "metadata";
+ public static final String VALUE_VECTOR_NAME = "value";
+
+ private final Field rootField;
+
+ /**
+ * Constructs a new VariantVector with the given name and allocator.
+ *
+ * @param name the name of the vector
+ * @param allocator the buffer allocator for memory management
+ */
+ public VariantVector(String name, BufferAllocator allocator) {
+ super(
+ name,
+ allocator,
+ new StructVector(
+ name,
+ allocator,
+ FieldType.nullable(ArrowType.Struct.INSTANCE),
+ null,
+ AbstractStructVector.ConflictPolicy.CONFLICT_ERROR,
+ false));
+ rootField = createVariantField(name);
+ ((FieldVector) this.getUnderlyingVector())
+ .initializeChildrenFromFields(rootField.getChildren());
+ }
+
+ /**
+ * Creates a new VariantVector with the given name. The Variant Field schema has to be the same
+ * everywhere, otherwise ArrowBuffer loading might fail during serialization/deserialization and
+ * schema mismatches can occur. This includes CompleteType's VARIANT and VARIANT_REQUIRED types.
+ */
+ public static Field createVariantField(String name) {
+ return new Field(
+ name, new FieldType(true, VariantType.INSTANCE, null), createVariantChildFields());
+ }
+
+ /**
+ * Creates the child fields for the VariantVector. Metadata vector will be index 0 and value
+ * vector will be index 1.
+ */
+ public static List<Field> createVariantChildFields() {
+ return List.of(
+ new Field(METADATA_VECTOR_NAME, new FieldType(false, Binary.INSTANCE, null), null),
+ new Field(VALUE_VECTOR_NAME, new FieldType(false, Binary.INSTANCE, null), null));
+ }
+
+ @Override
+ public void initializeChildrenFromFields(List<Field> children) {
+ // No-op, as children are initialized in the constructor
+ }
+
+ @Override
+ public Field getField() {
+ return rootField;
+ }
+
+ public VarBinaryVector getMetadataVector() {
+ return getUnderlyingVector().getChild(METADATA_VECTOR_NAME, VarBinaryVector.class);
+ }
+
+ public VarBinaryVector getValueVector() {
+ return getUnderlyingVector().getChild(VALUE_VECTOR_NAME, VarBinaryVector.class);
+ }
+
+ @Override
+ public TransferPair makeTransferPair(ValueVector target) {
+ return new VariantTransferPair(this, (VariantVector) target);
+ }
+
+ @Override
+ public TransferPair getTransferPair(Field field, BufferAllocator allocator) {
+ return new VariantTransferPair(this, new VariantVector(field.getName(), allocator));
+ }
+
+ @Override
+ public TransferPair getTransferPair(Field field, BufferAllocator allocator, CallBack callBack) {
+ return getTransferPair(field, allocator);
+ }
+
+ @Override
+ public TransferPair getTransferPair(String ref, BufferAllocator allocator) {
+ return new VariantTransferPair(this, new VariantVector(ref, allocator));
+ }
+
+ @Override
+ public TransferPair getTransferPair(String ref, BufferAllocator allocator, CallBack callBack) {
+ return getTransferPair(ref, allocator);
+ }
+
+ @Override
+ public TransferPair getTransferPair(BufferAllocator allocator) {
+ return getTransferPair(this.getField().getName(), allocator);
+ }
+
+ @Override
+ public void copyFrom(int fromIndex, int thisIndex, ValueVector from) {
+ getUnderlyingVector()
+ .copyFrom(fromIndex, thisIndex, ((VariantVector) from).getUnderlyingVector());
+ }
+
+ @Override
+ public void copyFromSafe(int fromIndex, int thisIndex, ValueVector from) {
+ getUnderlyingVector()
+ .copyFromSafe(fromIndex, thisIndex, ((VariantVector) from).getUnderlyingVector());
+ }
+
+ @Override
+ public Object getObject(int index) {
+ return getUnderlyingVector().getObject(index);
+ }
Review Comment:
@jbonofre, I was thinking about only the ~~`arrow-variant`~~ `parquet-variant` module that was created for the sole purpose of having the variant binary format related code in one place so it is easy to use as a dependency. It contains 10 prod classes all together plus test code. I do agree to wrap this around to extend and avoid exposing its classes in the Arrow API.
I don't think it would be better to duplicate the code instead of using it.
--
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-946: Add Variant extension type support [arrow-java]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #947:
URL: https://github.com/apache/arrow-java/pull/947#discussion_r2715822404
##########
vector/src/main/java/org/apache/arrow/vector/extension/VariantVector.java:
##########
@@ -0,0 +1,306 @@
+/*
+ * 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 java.util.List;
+import org.apache.arrow.memory.BufferAllocator;
+import org.apache.arrow.memory.util.hash.ArrowBufHasher;
+import org.apache.arrow.vector.BitVectorHelper;
+import org.apache.arrow.vector.ExtensionTypeVector;
+import org.apache.arrow.vector.FieldVector;
+import org.apache.arrow.vector.ValueVector;
+import org.apache.arrow.vector.VarBinaryVector;
+import org.apache.arrow.vector.complex.AbstractStructVector;
+import org.apache.arrow.vector.complex.StructVector;
+import org.apache.arrow.vector.complex.reader.FieldReader;
+import org.apache.arrow.vector.holders.NullableVariantHolder;
+import org.apache.arrow.vector.holders.VariantHolder;
+import org.apache.arrow.vector.types.pojo.ArrowType;
+import org.apache.arrow.vector.types.pojo.ArrowType.Binary;
+import org.apache.arrow.vector.types.pojo.Field;
+import org.apache.arrow.vector.types.pojo.FieldType;
+import org.apache.arrow.vector.util.CallBack;
+import org.apache.arrow.vector.util.TransferPair;
+
+public class VariantVector extends ExtensionTypeVector<StructVector> {
+
+ public static final String METADATA_VECTOR_NAME = "metadata";
+ public static final String VALUE_VECTOR_NAME = "value";
+
+ private final Field rootField;
+
+ /**
+ * Constructs a new VariantVector with the given name and allocator.
+ *
+ * @param name the name of the vector
+ * @param allocator the buffer allocator for memory management
+ */
+ public VariantVector(String name, BufferAllocator allocator) {
+ super(
+ name,
+ allocator,
+ new StructVector(
+ name,
+ allocator,
+ FieldType.nullable(ArrowType.Struct.INSTANCE),
+ null,
+ AbstractStructVector.ConflictPolicy.CONFLICT_ERROR,
+ false));
+ rootField = createVariantField(name);
+ ((FieldVector) this.getUnderlyingVector())
+ .initializeChildrenFromFields(rootField.getChildren());
+ }
+
+ /**
+ * Creates a new VariantVector with the given name. The Variant Field schema has to be the same
+ * everywhere, otherwise ArrowBuffer loading might fail during serialization/deserialization and
+ * schema mismatches can occur. This includes CompleteType's VARIANT and VARIANT_REQUIRED types.
+ */
+ public static Field createVariantField(String name) {
+ return new Field(
+ name, new FieldType(true, VariantType.INSTANCE, null), createVariantChildFields());
+ }
+
+ /**
+ * Creates the child fields for the VariantVector. Metadata vector will be index 0 and value
+ * vector will be index 1.
+ */
+ public static List<Field> createVariantChildFields() {
+ return List.of(
+ new Field(METADATA_VECTOR_NAME, new FieldType(false, Binary.INSTANCE, null), null),
+ new Field(VALUE_VECTOR_NAME, new FieldType(false, Binary.INSTANCE, null), null));
+ }
+
+ @Override
+ public void initializeChildrenFromFields(List<Field> children) {
+ // No-op, as children are initialized in the constructor
+ }
+
+ @Override
+ public Field getField() {
+ return rootField;
+ }
+
+ public VarBinaryVector getMetadataVector() {
+ return getUnderlyingVector().getChild(METADATA_VECTOR_NAME, VarBinaryVector.class);
+ }
+
+ public VarBinaryVector getValueVector() {
+ return getUnderlyingVector().getChild(VALUE_VECTOR_NAME, VarBinaryVector.class);
+ }
+
+ @Override
+ public TransferPair makeTransferPair(ValueVector target) {
+ return new VariantTransferPair(this, (VariantVector) target);
+ }
+
+ @Override
+ public TransferPair getTransferPair(Field field, BufferAllocator allocator) {
+ return new VariantTransferPair(this, new VariantVector(field.getName(), allocator));
+ }
+
+ @Override
+ public TransferPair getTransferPair(Field field, BufferAllocator allocator, CallBack callBack) {
+ return getTransferPair(field, allocator);
+ }
+
+ @Override
+ public TransferPair getTransferPair(String ref, BufferAllocator allocator) {
+ return new VariantTransferPair(this, new VariantVector(ref, allocator));
+ }
+
+ @Override
+ public TransferPair getTransferPair(String ref, BufferAllocator allocator, CallBack callBack) {
+ return getTransferPair(ref, allocator);
+ }
+
+ @Override
+ public TransferPair getTransferPair(BufferAllocator allocator) {
+ return getTransferPair(this.getField().getName(), allocator);
+ }
+
+ @Override
+ public void copyFrom(int fromIndex, int thisIndex, ValueVector from) {
+ getUnderlyingVector()
+ .copyFrom(fromIndex, thisIndex, ((VariantVector) from).getUnderlyingVector());
+ }
+
+ @Override
+ public void copyFromSafe(int fromIndex, int thisIndex, ValueVector from) {
+ getUnderlyingVector()
+ .copyFromSafe(fromIndex, thisIndex, ((VariantVector) from).getUnderlyingVector());
+ }
+
+ @Override
+ public Object getObject(int index) {
+ return getUnderlyingVector().getObject(index);
+ }
Review Comment:
Hmm, yeah, `arrow-parquet-variant` might be OK to isolate the dependency.
--
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