From de95e9c7f191fdb411af495c1f840e8224be86e7 Mon Sep 17 00:00:00 2001 From: Vibhatha Lakmal Abeykoon Date: Wed, 11 Sep 2024 14:02:13 +0530 Subject: [PATCH] fix: cleaning vector module v2 --- .../org/apache/arrow/vector/ZeroVector.java | 1 + .../vector/compare/RangeEqualsVisitor.java | 22 ++++++++++++------- .../vector/compare/TypeEqualsVisitor.java | 19 +++++++++++++--- .../complex/impl/ComplexWriterImpl.java | 3 +++ .../complex/impl/StructOrListWriterImpl.java | 15 +++++++++++++ .../dictionary/DictionaryHashTable.java | 1 + .../dictionary/StructSubfieldEncoder.java | 2 +- .../arrow/vector/ipc/ArrowStreamReader.java | 1 + .../apache/arrow/vector/ipc/ArrowWriter.java | 9 +------- .../arrow/vector/ipc/JsonFileWriter.java | 14 ++++++------ .../apache/arrow/vector/ipc/WriteChannel.java | 3 --- .../arrow/vector/ipc/message/ArrowBlock.java | 2 +- .../arrow/vector/ipc/message/ArrowBuffer.java | 2 +- .../ipc/message/ArrowDictionaryBatch.java | 3 +++ .../arrow/vector/ipc/message/ArrowFooter.java | 2 +- .../vector/ipc/message/ArrowRecordBatch.java | 8 ++++--- 16 files changed, 71 insertions(+), 36 deletions(-) diff --git a/java/vector/src/main/java/org/apache/arrow/vector/ZeroVector.java b/java/vector/src/main/java/org/apache/arrow/vector/ZeroVector.java index c838de60d841a..8e3d487bbaa09 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/ZeroVector.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/ZeroVector.java @@ -57,6 +57,7 @@ public ZeroVector(Field field) { } @Deprecated + @SuppressWarnings("InlineMeInliner") public ZeroVector() {} @Override diff --git a/java/vector/src/main/java/org/apache/arrow/vector/compare/RangeEqualsVisitor.java b/java/vector/src/main/java/org/apache/arrow/vector/compare/RangeEqualsVisitor.java index ed51f748af577..23157528a6197 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/compare/RangeEqualsVisitor.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/compare/RangeEqualsVisitor.java @@ -478,12 +478,15 @@ protected boolean compareBaseVariableWidthVectors(Range range) { int offsetWidth = BaseVariableWidthVector.OFFSET_WIDTH; if (!isNull) { - final int startIndexLeft = leftVector.getOffsetBuffer().getInt(leftIndex * offsetWidth); - final int endIndexLeft = leftVector.getOffsetBuffer().getInt((leftIndex + 1) * offsetWidth); + final int startIndexLeft = + leftVector.getOffsetBuffer().getInt((long) leftIndex * offsetWidth); + final int endIndexLeft = + leftVector.getOffsetBuffer().getInt((long) (leftIndex + 1) * offsetWidth); - final int startIndexRight = rightVector.getOffsetBuffer().getInt(rightIndex * offsetWidth); + final int startIndexRight = + rightVector.getOffsetBuffer().getInt((long) rightIndex * offsetWidth); final int endIndexRight = - rightVector.getOffsetBuffer().getInt((rightIndex + 1) * offsetWidth); + rightVector.getOffsetBuffer().getInt((long) (rightIndex + 1) * offsetWidth); int ret = ByteFunctionHelpers.equal( @@ -657,12 +660,15 @@ protected boolean compareListVectors(Range range) { int offsetWidth = BaseRepeatedValueVector.OFFSET_WIDTH; if (!isNull) { - final int startIndexLeft = leftVector.getOffsetBuffer().getInt(leftIndex * offsetWidth); - final int endIndexLeft = leftVector.getOffsetBuffer().getInt((leftIndex + 1) * offsetWidth); + final int startIndexLeft = + leftVector.getOffsetBuffer().getInt((long) leftIndex * offsetWidth); + final int endIndexLeft = + leftVector.getOffsetBuffer().getInt((long) (leftIndex + 1) * offsetWidth); - final int startIndexRight = rightVector.getOffsetBuffer().getInt(rightIndex * offsetWidth); + final int startIndexRight = + rightVector.getOffsetBuffer().getInt((long) rightIndex * offsetWidth); final int endIndexRight = - rightVector.getOffsetBuffer().getInt((rightIndex + 1) * offsetWidth); + rightVector.getOffsetBuffer().getInt((long) (rightIndex + 1) * offsetWidth); if ((endIndexLeft - startIndexLeft) != (endIndexRight - startIndexRight)) { return false; diff --git a/java/vector/src/main/java/org/apache/arrow/vector/compare/TypeEqualsVisitor.java b/java/vector/src/main/java/org/apache/arrow/vector/compare/TypeEqualsVisitor.java index ce92b22ef61c9..ce9cb117ed81f 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/compare/TypeEqualsVisitor.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/compare/TypeEqualsVisitor.java @@ -62,10 +62,23 @@ public TypeEqualsVisitor(ValueVector right, boolean checkName, boolean checkMeta } /** Check type equals without passing IN param in VectorVisitor. */ - public boolean equals(ValueVector left) { + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (!(obj instanceof ValueVector)) { + return false; + } + ValueVector left = (ValueVector) obj; return left.accept(this, null); } + @Override + public int hashCode() { + return Objects.hash(right, checkName, checkMetadata); + } + @Override public Boolean visit(BaseFixedWidthVector left, Void value) { return compareField(left.getField(), right.getField()); @@ -138,12 +151,12 @@ public Boolean visit(LargeListViewVector left, Void value) { private boolean compareField(Field leftField, Field rightField) { - if (leftField == rightField) { + if (leftField.equals(rightField)) { return true; } return (!checkName || Objects.equals(leftField.getName(), rightField.getName())) - && Objects.equals(leftField.isNullable(), rightField.isNullable()) + && (leftField.isNullable() == rightField.isNullable()) && Objects.equals(leftField.getType(), rightField.getType()) && Objects.equals(leftField.getDictionary(), rightField.getDictionary()) && (!checkMetadata || Objects.equals(leftField.getMetadata(), rightField.getMetadata())) diff --git a/java/vector/src/main/java/org/apache/arrow/vector/complex/impl/ComplexWriterImpl.java b/java/vector/src/main/java/org/apache/arrow/vector/complex/impl/ComplexWriterImpl.java index f3e48aa050e30..07eb5ad0dcdf9 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/complex/impl/ComplexWriterImpl.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/complex/impl/ComplexWriterImpl.java @@ -37,7 +37,10 @@ public class ComplexWriterImpl extends AbstractFieldWriter implements ComplexWri Mode mode = Mode.INIT; private final String name; + + @SuppressWarnings("UnusedVariable") private final boolean unionEnabled; + private final NullableStructWriterFactory nullableStructWriterFactory; private enum Mode { diff --git a/java/vector/src/main/java/org/apache/arrow/vector/complex/impl/StructOrListWriterImpl.java b/java/vector/src/main/java/org/apache/arrow/vector/complex/impl/StructOrListWriterImpl.java index 7dbcbf8babe00..74318222c5fdb 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/complex/impl/StructOrListWriterImpl.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/complex/impl/StructOrListWriterImpl.java @@ -51,6 +51,7 @@ public StructOrListWriterImpl(final BaseWriter.ListWriter writer) { } /** Start writing to either the list or the struct. */ + @Override public void start() { if (struct != null) { struct.start(); @@ -60,6 +61,7 @@ public void start() { } /** Finish writing to the list or struct. */ + @Override public void end() { if (struct != null) { struct.end(); @@ -69,6 +71,7 @@ public void end() { } /** Creates a new writer for a struct with the given name. */ + @Override public StructOrListWriter struct(final String name) { assert struct != null; return new StructOrListWriterImpl(struct.struct(name)); @@ -81,6 +84,7 @@ public StructOrListWriter struct(final String name) { * @deprecated use {@link #listOfStruct(String)} instead. */ @Deprecated + @SuppressWarnings({"InlineMeValidator", "InlineMeSuggester"}) public StructOrListWriter listoftstruct(final String name) { return listOfStruct(name); } @@ -90,48 +94,59 @@ public StructOrListWriter listoftstruct(final String name) { * * @param name Unused. */ + @Override public StructOrListWriter listOfStruct(final String name) { assert list != null; return new StructOrListWriterImpl(list.struct()); } + @Override public StructOrListWriter list(final String name) { assert struct != null; return new StructOrListWriterImpl(struct.list(name)); } + @Override public boolean isStructWriter() { return struct != null; } + @Override public boolean isListWriter() { return list != null; } + @Override public VarCharWriter varChar(final String name) { return (struct != null) ? struct.varChar(name) : list.varChar(); } + @Override public IntWriter integer(final String name) { return (struct != null) ? struct.integer(name) : list.integer(); } + @Override public BigIntWriter bigInt(final String name) { return (struct != null) ? struct.bigInt(name) : list.bigInt(); } + @Override public Float4Writer float4(final String name) { return (struct != null) ? struct.float4(name) : list.float4(); } + @Override public Float8Writer float8(final String name) { return (struct != null) ? struct.float8(name) : list.float8(); } + @Override public BitWriter bit(final String name) { return (struct != null) ? struct.bit(name) : list.bit(); } + @Override public VarBinaryWriter binary(final String name) { return (struct != null) ? struct.varBinary(name) : list.varBinary(); } diff --git a/java/vector/src/main/java/org/apache/arrow/vector/dictionary/DictionaryHashTable.java b/java/vector/src/main/java/org/apache/arrow/vector/dictionary/DictionaryHashTable.java index 57faf51845c4e..73ec406c7fbbb 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/dictionary/DictionaryHashTable.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/dictionary/DictionaryHashTable.java @@ -246,6 +246,7 @@ public int hashCode() { return hash; } + @Override public final boolean equals(Object o) { if (!(o instanceof DictionaryHashTable.Entry)) { return false; diff --git a/java/vector/src/main/java/org/apache/arrow/vector/dictionary/StructSubfieldEncoder.java b/java/vector/src/main/java/org/apache/arrow/vector/dictionary/StructSubfieldEncoder.java index dc25bc32685dd..359583ac483a7 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/dictionary/StructSubfieldEncoder.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/dictionary/StructSubfieldEncoder.java @@ -80,7 +80,7 @@ private static StructVector cloneVector(StructVector vector, BufferAllocator all StructVector cloned = (StructVector) fieldType.createNewSingleVector( - vector.getField().getName(), allocator, /*schemaCallback=*/ null); + vector.getField().getName(), allocator, /* schemaCallBack= */ null); final ArrowFieldNode fieldNode = new ArrowFieldNode(vector.getValueCount(), vector.getNullCount()); diff --git a/java/vector/src/main/java/org/apache/arrow/vector/ipc/ArrowStreamReader.java b/java/vector/src/main/java/org/apache/arrow/vector/ipc/ArrowStreamReader.java index 69811dc71727c..94eec4be94050 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/ipc/ArrowStreamReader.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/ipc/ArrowStreamReader.java @@ -139,6 +139,7 @@ protected void closeReadSource() throws IOException { * @return true if a batch was read, false on EOS * @throws IOException on error */ + @Override public boolean loadNextBatch() throws IOException { prepareLoadNextBatch(); MessageResult result = messageReader.readNext(); diff --git a/java/vector/src/main/java/org/apache/arrow/vector/ipc/ArrowWriter.java b/java/vector/src/main/java/org/apache/arrow/vector/ipc/ArrowWriter.java index c0f2b113bcb54..e8ea452c0322b 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/ipc/ArrowWriter.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/ipc/ArrowWriter.java @@ -155,15 +155,8 @@ protected void writeDictionaryBatch(Dictionary dictionary) throws IOException { VectorUnloader unloader = new VectorUnloader(dictRoot, /*includeNullCount*/ true, this.codec, /*alignBuffers*/ true); ArrowRecordBatch batch = unloader.getRecordBatch(); - ArrowDictionaryBatch dictionaryBatch = new ArrowDictionaryBatch(id, batch, false); - try { + try (ArrowDictionaryBatch dictionaryBatch = new ArrowDictionaryBatch(id, batch, false)) { writeDictionaryBatch(dictionaryBatch); - } finally { - try { - dictionaryBatch.close(); - } catch (Exception e) { - throw new RuntimeException("Error occurred while closing dictionary.", e); - } } } diff --git a/java/vector/src/main/java/org/apache/arrow/vector/ipc/JsonFileWriter.java b/java/vector/src/main/java/org/apache/arrow/vector/ipc/JsonFileWriter.java index 68700fe6afd25..a3fb2be1a57c8 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/ipc/JsonFileWriter.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/ipc/JsonFileWriter.java @@ -26,6 +26,7 @@ import java.io.File; import java.io.IOException; import java.math.BigDecimal; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -248,14 +249,14 @@ private void writeFromVectorIntoJson(Field field, FieldVector vector) throws IOE // writing views ArrowBuf viewBuffer = vectorBuffers.get(1); List dataBuffers = vectorBuffers.subList(v + 1, vectorBuffers.size()); - writeValueToViewGenerator(bufferType, viewBuffer, dataBuffers, vector, i); + writeValueToViewGenerator(viewBuffer, dataBuffers, vector, i); } else if (bufferType.equals(VARIADIC_DATA_BUFFERS) && (vector.getMinorType() == MinorType.VIEWVARCHAR || vector.getMinorType() == MinorType.VIEWVARBINARY)) { ArrowBuf viewBuffer = vectorBuffers.get(1); // check if this is v-1 List dataBuffers = vectorBuffers.subList(v, vectorBuffers.size()); if (!dataBuffers.isEmpty()) { - writeValueToDataBufferGenerator(bufferType, viewBuffer, dataBuffers, vector); + writeValueToDataBufferGenerator(bufferType, viewBuffer, dataBuffers); // The variadic buffers are written at once and doesn't require iterating for // each index. // So, break the loop. @@ -350,7 +351,6 @@ private byte[] getView(final ArrowBuf viewBuffer, final List dataBuffe } private void writeValueToViewGenerator( - BufferType bufferType, ArrowBuf viewBuffer, List dataBuffers, FieldVector vector, @@ -383,7 +383,7 @@ private void writeValueToViewGenerator( } else { generator.writeFieldName("INLINED"); if (vector.getMinorType() == MinorType.VIEWVARCHAR) { - generator.writeString(new String(b, "UTF-8")); + generator.writeString(new String(b, StandardCharsets.UTF_8)); } else { generator.writeString(Hex.encodeHexString(b)); } @@ -392,7 +392,7 @@ private void writeValueToViewGenerator( } private void writeValueToDataBufferGenerator( - BufferType bufferType, ArrowBuf viewBuffer, List dataBuffers, FieldVector vector) + BufferType bufferType, ArrowBuf viewBuffer, List dataBuffers) throws IOException { if (bufferType.equals(VARIADIC_DATA_BUFFERS)) { Preconditions.checkNotNull(viewBuffer); @@ -560,8 +560,8 @@ private void writeValueToGenerator( case VARCHAR: { Preconditions.checkNotNull(offsetBuffer); - byte[] b = (BaseVariableWidthVector.get(buffer, offsetBuffer, index)); - generator.writeString(new String(b, "UTF-8")); + byte[] b = BaseVariableWidthVector.get(buffer, offsetBuffer, index); + generator.writeString(new String(b, StandardCharsets.UTF_8)); break; } case DECIMAL: diff --git a/java/vector/src/main/java/org/apache/arrow/vector/ipc/WriteChannel.java b/java/vector/src/main/java/org/apache/arrow/vector/ipc/WriteChannel.java index eeb2eaf566d6e..fca6b13cd542e 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/ipc/WriteChannel.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/ipc/WriteChannel.java @@ -23,8 +23,6 @@ import org.apache.arrow.memory.ArrowBuf; import org.apache.arrow.vector.ipc.message.FBSerializable; import org.apache.arrow.vector.ipc.message.MessageSerializer; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * Wrapper around a WritableByteChannel that maintains the position as well adding some common @@ -37,7 +35,6 @@ *

Please note that objects of this class are not thread-safe. */ public class WriteChannel implements AutoCloseable { - private static final Logger LOGGER = LoggerFactory.getLogger(WriteChannel.class); private static final byte[] ZERO_BYTES = new byte[8]; diff --git a/java/vector/src/main/java/org/apache/arrow/vector/ipc/message/ArrowBlock.java b/java/vector/src/main/java/org/apache/arrow/vector/ipc/message/ArrowBlock.java index 455229cc6dda5..ff002794ed50f 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/ipc/message/ArrowBlock.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/ipc/message/ArrowBlock.java @@ -75,7 +75,7 @@ public boolean equals(Object obj) { if (obj == null) { return false; } - if (getClass() != obj.getClass()) { + if (!(obj instanceof ArrowBlock)) { return false; } ArrowBlock other = (ArrowBlock) obj; diff --git a/java/vector/src/main/java/org/apache/arrow/vector/ipc/message/ArrowBuffer.java b/java/vector/src/main/java/org/apache/arrow/vector/ipc/message/ArrowBuffer.java index cebddeb660e25..1ec9888bd798d 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/ipc/message/ArrowBuffer.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/ipc/message/ArrowBuffer.java @@ -62,7 +62,7 @@ public boolean equals(Object obj) { if (obj == null) { return false; } - if (getClass() != obj.getClass()) { + if (!(obj instanceof ArrowBuffer)) { return false; } ArrowBuffer other = (ArrowBuffer) obj; diff --git a/java/vector/src/main/java/org/apache/arrow/vector/ipc/message/ArrowDictionaryBatch.java b/java/vector/src/main/java/org/apache/arrow/vector/ipc/message/ArrowDictionaryBatch.java index cee76433ea4c7..208092c9461af 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/ipc/message/ArrowDictionaryBatch.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/ipc/message/ArrowDictionaryBatch.java @@ -16,6 +16,7 @@ */ package org.apache.arrow.vector.ipc.message; +import com.google.errorprone.annotations.InlineMe; import com.google.flatbuffers.FlatBufferBuilder; import org.apache.arrow.flatbuf.DictionaryBatch; import org.apache.arrow.flatbuf.MessageHeader; @@ -31,6 +32,7 @@ public class ArrowDictionaryBatch implements ArrowMessage { private final boolean isDelta; @Deprecated + @InlineMe(replacement = "this(dictionaryId, dictionary, false)") public ArrowDictionaryBatch(long dictionaryId, ArrowRecordBatch dictionary) { this(dictionaryId, dictionary, false); } @@ -46,6 +48,7 @@ public boolean isDelta() { return isDelta; } + @Override public byte getMessageType() { return MessageHeader.DictionaryBatch; } diff --git a/java/vector/src/main/java/org/apache/arrow/vector/ipc/message/ArrowFooter.java b/java/vector/src/main/java/org/apache/arrow/vector/ipc/message/ArrowFooter.java index bb2b87113faca..27ef82a603b2a 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/ipc/message/ArrowFooter.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/ipc/message/ArrowFooter.java @@ -190,7 +190,7 @@ public boolean equals(Object obj) { if (obj == null) { return false; } - if (getClass() != obj.getClass()) { + if (!(obj instanceof ArrowFooter)) { return false; } ArrowFooter other = (ArrowFooter) obj; diff --git a/java/vector/src/main/java/org/apache/arrow/vector/ipc/message/ArrowRecordBatch.java b/java/vector/src/main/java/org/apache/arrow/vector/ipc/message/ArrowRecordBatch.java index bc6bfa8c868f7..73b4c5313ffe4 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/ipc/message/ArrowRecordBatch.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/ipc/message/ArrowRecordBatch.java @@ -182,6 +182,7 @@ public ArrowRecordBatch( // this constructor is different from the public ones in that the reference manager's // retain method is not called, so the first dummy parameter is used // to distinguish this from the public constructor. + @SuppressWarnings("UnusedVariable") private ArrowRecordBatch( boolean dummy, int length, @@ -206,6 +207,7 @@ private ArrowRecordBatch( this.buffersLayout = Collections.unmodifiableList(arrowBuffers); } + @Override public byte getMessageType() { return org.apache.arrow.flatbuf.MessageHeader.RecordBatch; } @@ -261,9 +263,9 @@ public ArrowRecordBatch cloneWithTransfer(final BufferAllocator allocator) { buffers.stream() .map( buf -> - (buf.getReferenceManager() - .transferOwnership(buf, allocator) - .getTransferredBuffer()) + buf.getReferenceManager() + .transferOwnership(buf, allocator) + .getTransferredBuffer() .writerIndex(buf.writerIndex())) .collect(Collectors.toList()); close();