fix: cleaning vector module v6

vibhatha · Sep 13, 2024 · 7c1d829 · 7c1d829
1 parent ab93ca8
commit 7c1d829
Show file tree

Hide file tree

Showing 27 changed files with 88 additions and 45 deletions.
diff --git a/...er/jdbc/src/test/java/org/apache/arrow/adapter/jdbc/h2/JdbcToArrowVectorIteratorTest.java b/...er/jdbc/src/test/java/org/apache/arrow/adapter/jdbc/h2/JdbcToArrowVectorIteratorTest.java
@@ -27,10 +27,10 @@
 import static org.apache.arrow.adapter.jdbc.JdbcToArrowTestHelper.getLongValues;
 import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.io.IOException;
@@ -146,10 +146,10 @@ public void testVectorSchemaRootReuse(Table table, boolean reuseVectorSchemaRoot
 
         if (reuseVectorSchemaRoot) {
           // when reuse is enabled, different iterations are based on the same vector schema root.
-          assertTrue(prev == cur);
+          assertSame(prev, cur);
         } else {
           // when reuse is enabled, a new vector schema root is created in each iteration.
-          assertFalse(prev == cur);
+          assertNotEquals(prev, cur);
           if (batchCount < 3) {
             cur.close();
           }

diff --git a/java/flight/flight-core/src/main/java/org/apache/arrow/flight/CloseSessionResult.java b/java/flight/flight-core/src/main/java/org/apache/arrow/flight/CloseSessionResult.java
@@ -41,6 +41,7 @@ public static Status fromProtocol(Flight.CloseSessionResult.Status proto) {
       return values()[proto.getNumber()];
     }
 
+    @SuppressWarnings("EnumOrdinal")
     public Flight.CloseSessionResult.Status toProtocol() {
       return Flight.CloseSessionResult.Status.values()[ordinal()];
     }

diff --git a/java/flight/flight-core/src/main/java/org/apache/arrow/flight/DictionaryUtils.java b/java/flight/flight-core/src/main/java/org/apache/arrow/flight/DictionaryUtils.java
@@ -79,7 +79,7 @@ static Schema generateSchemaMessages(
               count);
       final VectorUnloader unloader = new VectorUnloader(dictRoot);
       try (final ArrowDictionaryBatch dictionaryBatch =
-              new ArrowDictionaryBatch(id, unloader.getRecordBatch());
+              new ArrowDictionaryBatch(id, unloader.getRecordBatch(), false);
           final ArrowMessage message = new ArrowMessage(dictionaryBatch, option)) {
         messageCallback.accept(message);
       }

diff --git a/java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightRuntimeException.java b/java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightRuntimeException.java
@@ -37,6 +37,11 @@ public CallStatus status() {
 
   @Override
   public String toString() {
+    return getMessage();
+  }
+
+  @Override
+  public String getMessage() {
     String s = getClass().getName();
     return String.format("%s: %s: %s", s, status.code(), status.description());
   }

diff --git a/...ight/flight-core/src/main/java/org/apache/arrow/flight/NoOpSessionOptionValueVisitor.java b/...ight/flight-core/src/main/java/org/apache/arrow/flight/NoOpSessionOptionValueVisitor.java
@@ -25,26 +25,31 @@
  */
 public class NoOpSessionOptionValueVisitor<T> implements SessionOptionValueVisitor<T> {
   /** A callback to handle SessionOptionValue containing a String. */
+  @Override
   public T visit(String value) {
     return null;
   }
 
   /** A callback to handle SessionOptionValue containing a boolean. */
+  @Override
   public T visit(boolean value) {
     return null;
   }
 
   /** A callback to handle SessionOptionValue containing a long. */
+  @Override
   public T visit(long value) {
     return null;
   }
 
   /** A callback to handle SessionOptionValue containing a double. */
+  @Override
   public T visit(double value) {
     return null;
   }
 
   /** A callback to handle SessionOptionValue containing an array of String. */
+  @Override
   public T visit(String[] value) {
     return null;
   }
@@ -55,6 +60,7 @@ public T visit(String[] value) {
    * <p>By convention, an attempt to set a valueless SessionOptionValue should attempt to unset or
    * clear the named option value on the server.
    */
+  @Override
   public T visit(Void value) {
     return null;
   }

diff --git a/java/flight/flight-core/src/main/java/org/apache/arrow/flight/ServerSessionMiddleware.java b/java/flight/flight-core/src/main/java/org/apache/arrow/flight/ServerSessionMiddleware.java
@@ -16,8 +16,10 @@
  */
 package org.apache.arrow.flight;
 
+import com.google.common.base.Splitter;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ConcurrentHashMap;
@@ -83,15 +85,15 @@ public ServerSessionMiddleware onCallStarted(
       if (it != null) {
         findIdCookie:
         for (final String headerValue : it) {
-          for (final String cookie : headerValue.split(" ;")) {
-            final String[] cookiePair = cookie.split("=");
-            if (cookiePair.length != 2) {
+          for (final String cookie : Splitter.on(" ;").split(headerValue)) {
+            final List<String> cookiePair = Splitter.on('=').splitToList(cookie);
+            if (cookiePair.size() != 2) {
               // Soft failure:  Ignore invalid cookie list field
               break;
             }
 
-            if (sessionCookieName.equals(cookiePair[0]) && cookiePair[1].length() > 0) {
-              sessionId = cookiePair[1];
+            if (sessionCookieName.equals(cookiePair.get(0)) && !cookiePair.get(1).isEmpty()) {
+              sessionId = cookiePair.get(1);
               break findIdCookie;
             }
           }

diff --git a/java/flight/flight-core/src/main/java/org/apache/arrow/flight/SessionOptionValue.java b/java/flight/flight-core/src/main/java/org/apache/arrow/flight/SessionOptionValue.java
@@ -38,7 +38,8 @@ public boolean isEmpty() {
     return false;
   }
 
-  private class SessionOptionValueToProtocolVisitor implements SessionOptionValueVisitor<Void> {
+  private static class SessionOptionValueToProtocolVisitor
+      implements SessionOptionValueVisitor<Void> {
     final Flight.SessionOptionValue.Builder b;
 
     SessionOptionValueToProtocolVisitor(Flight.SessionOptionValue.Builder b) {

diff --git a/java/flight/flight-core/src/main/java/org/apache/arrow/flight/SessionOptionValueFactory.java b/java/flight/flight-core/src/main/java/org/apache/arrow/flight/SessionOptionValueFactory.java
@@ -89,7 +89,7 @@ public boolean equals(Object o) {
       if (this == o) {
         return true;
       }
-      if (o == null || getClass() != o.getClass()) {
+      if (!(o instanceof SessionOptionValueString)) {
         return false;
       }
       SessionOptionValueString that = (SessionOptionValueString) o;
@@ -124,7 +124,7 @@ public boolean equals(Object o) {
       if (this == o) {
         return true;
       }
-      if (o == null || getClass() != o.getClass()) {
+      if (!(o instanceof SessionOptionValueBoolean)) {
         return false;
       }
       SessionOptionValueBoolean that = (SessionOptionValueBoolean) o;
@@ -159,7 +159,7 @@ public boolean equals(Object o) {
       if (this == o) {
         return true;
       }
-      if (o == null || getClass() != o.getClass()) {
+      if (!(o instanceof SessionOptionValueLong)) {
         return false;
       }
       SessionOptionValueLong that = (SessionOptionValueLong) o;
@@ -194,7 +194,7 @@ public boolean equals(Object o) {
       if (this == o) {
         return true;
       }
-      if (o == null || getClass() != o.getClass()) {
+      if (!(o instanceof SessionOptionValueDouble)) {
         return false;
       }
       SessionOptionValueDouble that = (SessionOptionValueDouble) o;
@@ -229,7 +229,7 @@ public boolean equals(Object o) {
       if (this == o) {
         return true;
       }
-      if (o == null || getClass() != o.getClass()) {
+      if (!(o instanceof SessionOptionValueStringList)) {
         return false;
       }
       SessionOptionValueStringList that = (SessionOptionValueStringList) o;
@@ -266,7 +266,7 @@ public boolean equals(Object o) {
       if (this == o) {
         return true;
       }
-      if (o == null || getClass() != o.getClass()) {
+      if (!(o instanceof SessionOptionValueEmpty)) {
         return false;
       }
       return true;

diff --git a/java/flight/flight-core/src/main/java/org/apache/arrow/flight/SetSessionOptionsResult.java b/java/flight/flight-core/src/main/java/org/apache/arrow/flight/SetSessionOptionsResult.java
@@ -45,12 +45,14 @@ static ErrorValue fromProtocol(Flight.SetSessionOptionsResult.ErrorValue s) {
       return values()[s.getNumber()];
     }
 
+    @SuppressWarnings("EnumOrdinal")
     Flight.SetSessionOptionsResult.ErrorValue toProtocol() {
       return Flight.SetSessionOptionsResult.ErrorValue.values()[ordinal()];
     }
   }
 
   /** Per-option extensible error response container. */
+  @SuppressWarnings("JavaLangClash")
   public static class Error {
     public ErrorValue value;
 
@@ -74,7 +76,7 @@ public boolean equals(Object o) {
       if (this == o) {
         return true;
       }
-      if (o == null || getClass() != o.getClass()) {
+      if (!(o instanceof Error)) {
         return false;
       }
       Error that = (Error) o;

diff --git a/java/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/AddWritableBuffer.java b/java/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/AddWritableBuffer.java
@@ -70,8 +70,8 @@ public class AddWritableBuffer {
       tmpBufChainOut = tmpBufChainOut2;
 
     } catch (Exception ex) {
-      new RuntimeException("Failed to initialize AddWritableBuffer, falling back to slow path", ex)
-          .printStackTrace();
+      throw new RuntimeException(
+          "Failed to initialize AddWritableBuffer, falling back to slow path", ex);
     }
 
     bufConstruct = tmpConstruct;

diff --git a/java/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/GetReadableBuffer.java b/java/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/GetReadableBuffer.java
@@ -47,8 +47,8 @@ public class GetReadableBuffer {
       tmpField = f;
       tmpClazz = clazz;
     } catch (Exception e) {
-      new RuntimeException("Failed to initialize GetReadableBuffer, falling back to slow path", e)
-          .printStackTrace();
+      throw new RuntimeException(
+          "Failed to initialize GetReadableBuffer, falling back to slow path", e);
     }
     READABLE_BUFFER = tmpField;
     BUFFER_INPUT_STREAM = tmpClazz;

diff --git a/java/flight/flight-core/src/test/java/org/apache/arrow/flight/TestFlightService.java b/java/flight/flight-core/src/test/java/org/apache/arrow/flight/TestFlightService.java
@@ -150,7 +150,9 @@ public FlightInfo getFlightInfo(CallContext context, FlightDescriptor descriptor
       Exception e =
           assertThrows(
               FlightRuntimeException.class, () -> client.getSchema(FlightDescriptor.path("test")));
-      assertEquals("No schema is present in FlightInfo", e.getMessage());
+      assertEquals(
+          "org.apache.arrow.flight.FlightRuntimeException: INVALID_ARGUMENT: No schema is present in FlightInfo",
+          e.getMessage());
     }
   }
 
@@ -211,7 +213,9 @@ public FlightInfo getFlightInfo(CallContext context, FlightDescriptor descriptor
               FlightRuntimeException.class,
               () ->
                   client.getInfo(FlightDescriptor.path("test"), new HeaderCallOption(callHeaders)));
-      assertEquals("http2 exception", e.getMessage());
+      assertEquals(
+          "org.apache.arrow.flight.FlightRuntimeException: INTERNAL: http2 exception",
+          e.getMessage());
     }
   }
 

diff --git a/...light-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcDriver.java b/...light-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcDriver.java
@@ -26,6 +26,7 @@
 import java.net.URI;
 import java.nio.charset.StandardCharsets;
 import java.sql.SQLException;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
@@ -277,7 +278,8 @@ Optional<Map<Object, Object>> getUrlsArgs(String url) throws SQLException {
 
   static Properties lowerCasePropertyKeys(final Properties properties) {
     final Properties resultProperty = new Properties();
-    properties.forEach((k, v) -> resultProperty.put(k.toString().toLowerCase(), v));
+    properties.forEach(
+        (k, v) -> resultProperty.put(k.toString().toLowerCase(Locale.getDefault()), v));
     return resultProperty;
   }
 }
diff --git a/.../apache/arrow/driver/jdbc/accessor/impl/numeric/ArrowFlightJdbcDecimalVectorAccessor.java b/.../apache/arrow/driver/jdbc/accessor/impl/numeric/ArrowFlightJdbcDecimalVectorAccessor.java
@@ -71,6 +71,7 @@ public String getString() {
   }
 
   @Override
+  @SuppressWarnings("BigDecimalEquals")
   public boolean getBoolean() {
     final BigDecimal value = this.getBigDecimal();
 

diff --git a/...ore/src/main/java/org/apache/arrow/driver/jdbc/utils/ArrowFlightConnectionConfigImpl.java b/...ore/src/main/java/org/apache/arrow/driver/jdbc/utils/ArrowFlightConnectionConfigImpl.java
@@ -18,6 +18,7 @@
 
 import java.util.Arrays;
 import java.util.HashMap;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Properties;
@@ -196,6 +197,7 @@ public Map<String, String> getHeaderAttributes() {
   }
 
   /** Custom {@link ConnectionProperty} for the {@link ArrowFlightConnectionConfigImpl}. */
+  @SuppressWarnings("Immutable")
   public enum ArrowFlightConnectionProperty implements ConnectionProperty {
     HOST("host", null, Type.STRING, true),
     PORT("port", null, Type.NUMBER, true),
@@ -241,7 +243,7 @@ public Object get(final Properties properties) {
       Preconditions.checkNotNull(properties, "Properties cannot be null.");
       Object value = properties.get(camelName);
       if (value == null) {
-        value = properties.get(camelName.toLowerCase());
+        value = properties.get(camelName.toLowerCase(Locale.getDefault()));
       }
       if (required) {
         if (value == null) {

diff --git a/...flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/ResultSetTest.java b/...flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/ResultSetTest.java
@@ -156,8 +156,6 @@ public void testShouldRunSelectQuerySettingMaxRowLimit() throws Exception {
 
   /**
    * Tests whether the {@link ArrowFlightJdbcDriver} fails upon attempting to run an invalid query.
-   *
-   * @throws Exception If the connection fails to be established.
    */
   @Test
   public void testShouldThrowExceptionUponAttemptingToExecuteAnInvalidSelectQuery() {
@@ -336,6 +334,7 @@ public void testShouldInterruptFlightStreamsIfQueryIsCancelledMidQuerying()
   }
 
   @Test
+  @SuppressWarnings("ThreadPriorityCheck")
   public void
       testShouldInterruptFlightStreamsIfQueryIsCancelledMidProcessingForTimeConsumingQueries()
           throws SQLException, InterruptedException {

diff --git a/...e/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcIntervalVectorAccessorTest.java b/...e/arrow/driver/jdbc/accessor/impl/calendar/ArrowFlightJdbcIntervalVectorAccessorTest.java
@@ -22,9 +22,11 @@
 import static org.hamcrest.CoreMatchers.is;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
+import com.google.common.base.Splitter;
 import java.time.Duration;
 import java.time.Period;
 import java.time.format.DateTimeParseException;
+import java.util.List;
 import java.util.function.Supplier;
 import java.util.stream.Stream;
 import org.apache.arrow.driver.jdbc.accessor.ArrowFlightJdbcAccessorFactory;
@@ -110,7 +112,7 @@ public static Stream<Arguments> data() {
                   int valueCount = 10;
                   vector.setValueCount(valueCount);
                   for (int i = 0; i < valueCount; i++) {
-                    vector.set(i, i + 1, (i + 1) * 10, (i + 1) * 100);
+                    vector.set(i, i + 1, (i + 1) * 10, (i + 1) * 100L);
                   }
                   return vector;
                 },
@@ -172,11 +174,11 @@ private String getStringOnVector(ValueVector vector, int index) {
       return formatIntervalYear(Period.parse(object.toString()));
     } else if (vector instanceof IntervalMonthDayNanoVector) {
       String iso8601IntervalString = ((PeriodDuration) object).toISO8601IntervalString();
-      String[] periodAndDuration = iso8601IntervalString.split("T");
-      if (periodAndDuration.length == 1) {
+      List<String> periodAndDuration = Splitter.on('T').splitToList(iso8601IntervalString);
+      if (periodAndDuration.size() == 1) {
         // If there is no 'T', then either Period or Duration is zero, and the other one will
         // successfully parse it
-        String periodOrDuration = periodAndDuration[0];
+        String periodOrDuration = periodAndDuration.get(0);
         try {
           return new PeriodDuration(Period.parse(periodOrDuration), Duration.ZERO)
               .toISO8601IntervalString();
@@ -188,8 +190,8 @@ private String getStringOnVector(ValueVector vector, int index) {
         // If there is a 'T', both Period and Duration are non-zero, and we just need to prepend the
         // 'PT' to the
         // duration for both to parse successfully
-        Period parse = Period.parse(periodAndDuration[0]);
-        Duration duration = Duration.parse("PT" + periodAndDuration[1]);
+        Period parse = Period.parse(periodAndDuration.get(0));
+        Duration duration = Duration.parse("PT" + periodAndDuration.get(1));
         return new PeriodDuration(parse, duration).toISO8601IntervalString();
       }
     }