fix: cleaning vector module (unexpected)

vibhatha · Sep 11, 2024 · 0e79ee9 · 0e79ee9
1 parent 2f91c9b
commit 0e79ee9
Show file tree

Hide file tree

Showing 26 changed files with 122 additions and 41 deletions.
diff --git a/java/pom.xml b/java/pom.xml
@@ -923,7 +923,7 @@ under the License.
             <configuration>
               <compilerArgs combine.children="append">
                 <arg>-XDcompilePolicy=simple</arg>
-                <arg>-Xplugin:ErrorProne -XepExcludedPaths:.*/(target/generated-source|format/src/main/java/org/apache/arrow/flatbuf)/.*</arg>
+                <arg>-Xplugin:ErrorProne -XepExcludedPaths:.*/(target/generated-source|target/generated-sources|format/src/main/java/org/apache/arrow/flatbuf)/.*</arg>
                 <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED</arg>
                 <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED</arg>
                 <arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED</arg>

diff --git a/java/vector/pom.xml b/java/vector/pom.xml
@@ -86,6 +86,11 @@ under the License.
       <artifactId>hamcrest</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>com.google.errorprone</groupId>
+      <artifactId>error_prone_annotations</artifactId>
+      <scope>compile</scope>
+    </dependency>
   </dependencies>
 
   <build>
@@ -118,15 +123,6 @@ under the License.
           </execution>
         </executions>
       </plugin>
-      <plugin>
-        <groupId>org.apache.maven.plugins</groupId>
-        <artifactId>maven-compiler-plugin</artifactId>
-        <configuration combine.children="append">
-          <compilerArgs>
-            <arg>-Werror</arg>
-          </compilerArgs>
-        </configuration>
-      </plugin>
       <plugin>
         <!-- generate sources from fmpp -->
         <groupId>org.apache.drill.tools</groupId>

diff --git a/java/vector/src/main/codegen/templates/ArrowType.java b/java/vector/src/main/codegen/templates/ArrowType.java
@@ -21,6 +21,7 @@
 
 package org.apache.arrow.vector.types.pojo;
 
+import com.google.errorprone.annotations.Immutable;
 import com.google.flatbuffers.FlatBufferBuilder;
 
 import java.util.Objects;
@@ -49,6 +50,8 @@
   @JsonSubTypes.Type(value = ArrowType.${type.name?remove_ending("_")}.class, name = "${type.name?remove_ending("_")?lower_case}"),
 </#list>
 })
+
+@Immutable
 public abstract class ArrowType {
 
   public static abstract class PrimitiveType extends ArrowType {

diff --git a/java/vector/src/main/java/module-info.java b/java/vector/src/main/java/module-info.java
@@ -48,6 +48,7 @@
   requires org.apache.arrow.memory.core;
   requires org.apache.commons.codec;
   requires org.slf4j;
+  requires com.google.errorprone.annotations;
 
   uses org.apache.arrow.vector.compression.CompressionCodec.Factory;
 }
diff --git a/java/vector/src/main/java/org/apache/arrow/vector/AllocationHelper.java b/java/vector/src/main/java/org/apache/arrow/vector/AllocationHelper.java
@@ -50,7 +50,7 @@ public static void allocatePrecomputedChildCount(
     if (v instanceof FixedWidthVector) {
       ((FixedWidthVector) v).allocateNew(valueCount);
     } else if (v instanceof VariableWidthVector) {
-      ((VariableWidthVector) v).allocateNew(valueCount * bytesPerValue, valueCount);
+      ((VariableWidthVector) v).allocateNew(valueCount * ((long) bytesPerValue), valueCount);
     } else if (v instanceof RepeatedFixedWidthVectorLike) {
       ((RepeatedFixedWidthVectorLike) v).allocateNew(valueCount, childValCount);
     } else if (v instanceof RepeatedVariableWidthVectorLike) {

diff --git a/java/vector/src/main/java/org/apache/arrow/vector/BaseLargeVariableWidthVector.java b/java/vector/src/main/java/org/apache/arrow/vector/BaseLargeVariableWidthVector.java
@@ -1006,6 +1006,7 @@ public void setIndexDefined(int index) {
    * @param index position of the element to set
    * @param length length of the element
    */
+  @Override
   public void setValueLengthSafe(int index, int length) {
     assert index >= 0;
     handleSafe(index, length);
@@ -1091,6 +1092,7 @@ public void set(int index, byte[] value, int start, int length) {
    * @param start start index in array of bytes
    * @param length length of data in array of bytes
    */
+  @Override
   public void setSafe(int index, byte[] value, int start, int length) {
     assert index >= 0;
     handleSafe(index, length);
@@ -1128,6 +1130,7 @@ public void set(int index, ByteBuffer value, int start, int length) {
    * @param start start index in ByteBuffer
    * @param length length of data in ByteBuffer
    */
+  @Override
   public void setSafe(int index, ByteBuffer value, int start, int length) {
     assert index >= 0;
     handleSafe(index, length);

diff --git a/java/vector/src/main/java/org/apache/arrow/vector/BaseVariableWidthVector.java b/java/vector/src/main/java/org/apache/arrow/vector/BaseVariableWidthVector.java
@@ -1129,6 +1129,7 @@ public void setSafe(int index, byte[] value) {
    * @param start start index in array of bytes
    * @param length length of data in array of bytes
    */
+  @Override
   public void set(int index, byte[] value, int start, int length) {
     assert index >= 0;
     fillHoles(index);
@@ -1146,6 +1147,7 @@ public void set(int index, byte[] value, int start, int length) {
    * @param start start index in array of bytes
    * @param length length of data in array of bytes
    */
+  @Override
   public void setSafe(int index, byte[] value, int start, int length) {
     assert index >= 0;
     handleSafe(index, length);
@@ -1183,6 +1185,7 @@ public void set(int index, ByteBuffer value, int start, int length) {
    * @param start start index in ByteBuffer
    * @param length length of data in ByteBuffer
    */
+  @Override
   public void setSafe(int index, ByteBuffer value, int start, int length) {
     assert index >= 0;
     handleSafe(index, length);

diff --git a/java/vector/src/main/java/org/apache/arrow/vector/BaseVariableWidthViewVector.java b/java/vector/src/main/java/org/apache/arrow/vector/BaseVariableWidthViewVector.java
@@ -401,7 +401,7 @@ private void setReaderAndWriterIndex() {
       viewBuffer.writerIndex(0);
     } else {
       validityBuffer.writerIndex(getValidityBufferSizeFromCount(valueCount));
-      viewBuffer.writerIndex(valueCount * ELEMENT_SIZE);
+      viewBuffer.writerIndex((long) valueCount * ELEMENT_SIZE);
     }
   }
 
@@ -619,7 +619,7 @@ public void reallocValidityBuffer() {
   }
 
   private long computeValidityBufferSize(int valueCount) {
-    return (valueCount + 7) / 8;
+    return (valueCount + 7L) / 8;
   }
 
   /**
@@ -941,7 +941,7 @@ private void splitAndTransferViewBufferAndDataBuffer(
     }
 
     // allocate target view buffer
-    target.viewBuffer = target.allocator.buffer(length * ELEMENT_SIZE);
+    target.viewBuffer = target.allocator.buffer(length * ((long) ELEMENT_SIZE));
 
     for (int i = startIndex; i < startIndex + length; i++) {
       final int stringLength = getValueLength(i);
@@ -1153,6 +1153,7 @@ public int getValueLength(int index) {
    * @param index position of the element to set
    * @param value array of bytes to write
    */
+  @Override
   public void set(int index, byte[] value) {
     assert index >= 0;
     BitVectorHelper.setBit(validityBuffer, index);
@@ -1185,6 +1186,7 @@ public void setSafe(int index, byte[] value) {
    * @param start start index in an array of bytes
    * @param length length of data in an array of bytes
    */
+  @Override
   public void set(int index, byte[] value, int start, int length) {
     assert index >= 0;
     BitVectorHelper.setBit(validityBuffer, index);
@@ -1201,6 +1203,7 @@ public void set(int index, byte[] value, int start, int length) {
    * @param start start index in an array of bytes
    * @param length length of data in an array of bytes
    */
+  @Override
   public void setSafe(int index, byte[] value, int start, int length) {
     assert index >= 0;
     handleSafe(index, length);
@@ -1217,10 +1220,20 @@ public void setSafe(int index, byte[] value, int start, int length) {
    * @param start start index in ByteBuffer
    * @param length length of data in ByteBuffer
    */
+  @Override
   public void set(int index, ByteBuffer value, int start, int length) {
     assert index >= 0;
     BitVectorHelper.setBit(validityBuffer, index);
-    setBytes(index, value.array(), start, length);
+    if (value.hasArray()) {
+      setBytes(index, value.array(), value.arrayOffset() + start, length);
+    } else {
+      byte[] bytes = new byte[length];
+      int originalPosition = value.position();
+      value.position(start);
+      value.get(bytes, 0, length);
+      value.position(originalPosition); // Restores the buffer position
+      setBytes(index, bytes, 0, length);
+    }
     lastSet = index;
   }
 
@@ -1233,11 +1246,21 @@ public void set(int index, ByteBuffer value, int start, int length) {
    * @param start start index in ByteBuffer
    * @param length length of data in ByteBuffer
    */
+  @Override
   public void setSafe(int index, ByteBuffer value, int start, int length) {
     assert index >= 0;
     handleSafe(index, length);
     BitVectorHelper.setBit(validityBuffer, index);
-    setBytes(index, value.array(), start, length);
+    if (value.hasArray()) {
+      setBytes(index, value.array(), value.arrayOffset() + start, length);
+    } else {
+      byte[] bytes = new byte[length];
+      int originalPosition = value.position();
+      value.position(start);
+      value.get(bytes, 0, length);
+      value.position(originalPosition); // Restores the buffer position
+      setBytes(index, bytes, 0, length);
+    }
     lastSet = index;
   }
 

diff --git a/java/vector/src/main/java/org/apache/arrow/vector/BitVectorHelper.java b/java/vector/src/main/java/org/apache/arrow/vector/BitVectorHelper.java
@@ -296,7 +296,7 @@ public static byte getBitsFromCurrentByte(
 
   /** Returns the byte at <code>index</code> from left-shifted by (8 - <code>offset</code>). */
   public static byte getBitsFromNextByte(ArrowBuf data, int index, int offset) {
-    return (byte) ((data.getByte(index) << (8 - offset)));
+    return (byte) (data.getByte(index) << (8 - offset));
   }
 
   /**
@@ -385,7 +385,7 @@ public static void concatBits(
     }
 
     // copy the first bit set
-    if (input1 != output) {
+    if (!input1.equals(output)) {
       MemoryUtil.copyMemory(input1.memoryAddress(), output.memoryAddress(), numBytes1);
     }
 

diff --git a/java/vector/src/main/java/org/apache/arrow/vector/LargeVarBinaryVector.java b/java/vector/src/main/java/org/apache/arrow/vector/LargeVarBinaryVector.java
@@ -93,6 +93,7 @@ public MinorType getMinorType() {
    * @param index position of element to get
    * @return array of bytes for non-null element, null otherwise
    */
+  @Override
   public byte[] get(int index) {
     assert index >= 0;
     if (isSet(index) == 0) {

diff --git a/java/vector/src/main/java/org/apache/arrow/vector/NullVector.java b/java/vector/src/main/java/org/apache/arrow/vector/NullVector.java
@@ -18,6 +18,7 @@
 
 import static org.apache.arrow.vector.complex.BaseRepeatedValueVector.DATA_VECTOR_NAME;
 
+import com.google.errorprone.annotations.InlineMe;
 import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
@@ -94,6 +95,15 @@ public NullVector(Field field, int valueCount) {
   }
 
   @Deprecated
+  @InlineMe(
+      replacement =
+          "this(new Field(DATA_VECTOR_NAME, FieldType.nullable(new ArrowType.Null()), null))",
+      imports = {
+        "DATA_VECTOR_NAME",
+        "org.apache.arrow.vector.types.pojo.ArrowType",
+        "org.apache.arrow.vector.types.pojo.Field",
+        "org.apache.arrow.vector.types.pojo.FieldType"
+      })
   public NullVector() {
     this(new Field(DATA_VECTOR_NAME, FieldType.nullable(new ArrowType.Null()), null));
   }
@@ -229,6 +239,7 @@ public List<ArrowBuf> getFieldBuffers() {
    *     vectors.
    */
   @Deprecated
+  @SuppressWarnings("InlineMeSuggester")
   @Override
   public List<BufferBacked> getFieldInnerVectors() {
     return Collections.emptyList();
@@ -336,7 +347,9 @@ public TransferImpl(String ref) {
 
     @Deprecated
     public TransferImpl() {
-      to = new NullVector();
+      to =
+          new NullVector(
+              new Field(DATA_VECTOR_NAME, FieldType.nullable(new ArrowType.Null()), null));
     }
 
     public TransferImpl(NullVector to) {

diff --git a/java/vector/src/main/java/org/apache/arrow/vector/TimeStampSecTZVector.java b/java/vector/src/main/java/org/apache/arrow/vector/TimeStampSecTZVector.java
@@ -126,6 +126,7 @@ public void get(int index, NullableTimeStampSecTZHolder holder) {
    * @param index position of element
    * @return element at given index
    */
+  @Override
   public Long getObject(int index) {
     if (isSet(index) == 0) {
       return null;

diff --git a/java/vector/src/main/java/org/apache/arrow/vector/VarBinaryVector.java b/java/vector/src/main/java/org/apache/arrow/vector/VarBinaryVector.java
@@ -94,6 +94,7 @@ public MinorType getMinorType() {
    * @param index position of element to get
    * @return array of bytes for non-null element, null otherwise
    */
+  @Override
   public byte[] get(int index) {
     assert index >= 0;
     if (NULL_CHECKING_ENABLED && isSet(index) == 0) {

diff --git a/java/vector/src/main/java/org/apache/arrow/vector/VarCharVector.java b/java/vector/src/main/java/org/apache/arrow/vector/VarCharVector.java
@@ -96,6 +96,7 @@ public MinorType getMinorType() {
    * @param index position of element to get
    * @return array of bytes for non-null element, null otherwise
    */
+  @Override
   public byte[] get(int index) {
     assert index >= 0;
     if (NULL_CHECKING_ENABLED && isSet(index) == 0) {

diff --git a/java/vector/src/main/java/org/apache/arrow/vector/VectorSchemaRoot.java b/java/vector/src/main/java/org/apache/arrow/vector/VectorSchemaRoot.java
@@ -347,11 +347,15 @@ public VectorSchemaRoot slice(int index, int length) {
     return new VectorSchemaRoot(sliceVectors);
   }
 
-  /** Determine if two VectorSchemaRoots are exactly equal. */
-  public boolean equals(VectorSchemaRoot other) {
-    if (other == null) {
+  @Override
+  public boolean equals(Object obj) {
+    if (this == obj) {
+      return true;
+    }
+    if (!(obj instanceof VectorSchemaRoot)) {
       return false;
     }
+    VectorSchemaRoot other = (VectorSchemaRoot) obj;
 
     if (!this.schema.equals(other.schema)) {
       return false;
@@ -372,6 +376,16 @@ public boolean equals(VectorSchemaRoot other) {
     return true;
   }
 
+  @Override
+  public int hashCode() {
+    int result = schema.hashCode();
+    result = 31 * result + rowCount;
+    for (FieldVector vector : fieldVectors) {
+      result = 31 * result + (vector != null ? vector.hashCode() : 0);
+    }
+    return result;
+  }
+
   /**
    * Determine if two VectorSchemaRoots are approximately equal using the given functions to
    * calculate difference between float/double values. Note that approx equals are in regards to

diff --git a/java/vector/src/main/java/org/apache/arrow/vector/ViewVarBinaryVector.java b/java/vector/src/main/java/org/apache/arrow/vector/ViewVarBinaryVector.java
@@ -18,6 +18,7 @@
 
 import static org.apache.arrow.vector.NullCheckingForGet.NULL_CHECKING_ENABLED;
 
+import com.google.errorprone.annotations.DoNotCall;
 import org.apache.arrow.memory.BufferAllocator;
 import org.apache.arrow.memory.ReusableBuffer;
 import org.apache.arrow.vector.complex.impl.ViewVarBinaryReaderImpl;
@@ -94,6 +95,7 @@ public MinorType getMinorType() {
    * @param index position of an element to get
    * @return array of bytes for a non-null element, null otherwise
    */
+  @Override
   public byte[] get(int index) {
     assert index >= 0;
     if (NULL_CHECKING_ENABLED && isSet(index) == 0) {
@@ -131,6 +133,7 @@ public byte[] getObject(int index) {
    * @param index position of an element to get
    * @param holder data holder to be populated by this function
    */
+  @DoNotCall("Always throws java.lang.UnsupportedOperationException")
   public void get(int index, NullableViewVarBinaryHolder holder) {
     // TODO: https://github.com/apache/arrow/issues/40936
     throw new UnsupportedOperationException("Unsupported operation");
@@ -149,6 +152,7 @@ public void get(int index, NullableViewVarBinaryHolder holder) {
    * @param index position of the element to set
    * @param holder holder that carries data buffer.
    */
+  @DoNotCall("Always throws java.lang.UnsupportedOperationException")
   public void set(int index, ViewVarBinaryHolder holder) {
     // TODO: https://github.com/apache/arrow/issues/40936
     throw new UnsupportedOperationException("Unsupported operation");
@@ -161,6 +165,7 @@ public void set(int index, ViewVarBinaryHolder holder) {
    * @param index position of the element to set
    * @param holder holder that carries data buffer.
    */
+  @DoNotCall("Always throws java.lang.UnsupportedOperationException")
   public void setSafe(int index, ViewVarBinaryHolder holder) {
     // TODO: https://github.com/apache/arrow/issues/40936
     throw new UnsupportedOperationException("Unsupported operation");
@@ -173,6 +178,7 @@ public void setSafe(int index, ViewVarBinaryHolder holder) {
    * @param index position of the element to set
    * @param holder holder that carries data buffer.
    */
+  @DoNotCall("Always throws java.lang.UnsupportedOperationException")
   public void set(int index, NullableViewVarBinaryHolder holder) {
     // TODO: https://github.com/apache/arrow/issues/40936
     throw new UnsupportedOperationException("Unsupported operation");
@@ -185,6 +191,7 @@ public void set(int index, NullableViewVarBinaryHolder holder) {
    * @param index position of the element to set
    * @param holder holder that carries data buffer.
    */
+  @DoNotCall("Always throws java.lang.UnsupportedOperationException")
   public void setSafe(int index, NullableViewVarBinaryHolder holder) {
     // TODO: https://github.com/apache/arrow/issues/40936
     throw new UnsupportedOperationException("Unsupported operation");