Skip to content

Commit

Permalink
fix: address reviews v2
Browse files Browse the repository at this point in the history
  • Loading branch information
vibhatha committed Jan 22, 2024
1 parent 7175c9d commit 220c6d4
Show file tree
Hide file tree
Showing 13 changed files with 37 additions and 45 deletions.
1 change: 1 addition & 0 deletions java/flight/flight-core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@
<dependency>
<groupId>com.google.protobuf</groupId>
<artifactId>protobuf-java-util</artifactId>
<version>3.23.1</version>
</dependency>
<dependency>
<groupId>io.grpc</groupId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public class AddWritableBuffer {
tmpBufChainOut = tmpBufChainOut2;

} catch (Exception ex) {
throw new RuntimeException("Failed to initialize AddWritableBuffer, falling back to slow path", ex);
new RuntimeException("Failed to initialize AddWritableBuffer, falling back to slow path", ex).printStackTrace();
}

bufConstruct = tmpConstruct;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public class GetReadableBuffer {
tmpField = f;
tmpClazz = clazz;
} catch (Exception e) {
throw new RuntimeException("Failed to initialize GetReadableBuffer, falling back to slow path", e);
new RuntimeException("Failed to initialize GetReadableBuffer, falling back to slow path", e).printStackTrace();
}
READABLE_BUFFER = tmpField;
BUFFER_INPUT_STREAM = tmpClazz;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public void doPutUncaught() {
Assertions.assertEquals(FlightStatusCode.OK, status.code());
Assertions.assertNull(status.cause());
Assertions.assertNotNull(err);
Assertions.assertEquals(new RuntimeException("test").getMessage(), err.getMessage());
Assertions.assertEquals("test", err.getMessage());
});
}

Expand All @@ -119,7 +119,7 @@ public void listFlightsUncaught() {
Assertions.assertEquals(FlightStatusCode.OK, status.code());
Assertions.assertNull(status.cause());
Assertions.assertNotNull(err);
Assertions.assertEquals(new RuntimeException("test").getMessage(), err.getMessage());
Assertions.assertEquals("test", err.getMessage());
});
}

Expand All @@ -134,7 +134,7 @@ public void doActionUncaught() {
Assertions.assertEquals(FlightStatusCode.OK, status.code());
Assertions.assertNull(status.cause());
Assertions.assertNotNull(err);
Assertions.assertEquals(new RuntimeException("test").getMessage(), err.getMessage());
Assertions.assertEquals("test", err.getMessage());
});
}

Expand All @@ -149,7 +149,7 @@ public void listActionsUncaught() {
Assertions.assertEquals(FlightStatusCode.OK, status.code());
Assertions.assertNull(status.cause());
Assertions.assertNotNull(err);
Assertions.assertEquals(new RuntimeException("test").getMessage(), err.getMessage());
Assertions.assertEquals("test", err.getMessage());
});
}

Expand Down Expand Up @@ -184,7 +184,7 @@ public void doGetUncaught() {
Assertions.assertEquals(FlightStatusCode.OK, status.code());
Assertions.assertNull(status.cause());
Assertions.assertNotNull(err);
Assertions.assertEquals(new RuntimeException("test").getMessage(), err.getMessage());
Assertions.assertEquals("test", err.getMessage());
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import java.net.URI;
import java.nio.charset.StandardCharsets;
import java.sql.SQLException;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
Expand All @@ -44,8 +43,6 @@
import org.apache.calcite.avatica.Meta;
import org.apache.calcite.avatica.UnregisteredDriver;

import com.google.common.base.Ascii;
import com.google.common.base.Splitter;

/**
* JDBC driver for querying data from an Apache Arrow Flight server.
Expand Down Expand Up @@ -103,6 +100,7 @@ protected String getFactoryClassName(final JdbcVersion jdbcVersion) {
}

@Override
@SuppressWarnings("StringSplitter")
protected DriverVersion createDriverVersion() {
if (version == null) {
final InputStream flightProperties = this.getClass().getResourceAsStream("/properties/flight.properties");
Expand All @@ -115,17 +113,17 @@ protected DriverVersion createDriverVersion() {

final String parentName = properties.getProperty("org.apache.arrow.flight.name");
final String parentVersion = properties.getProperty("org.apache.arrow.flight.version");
final List<String> pVersion = Splitter.on('.').splitToList(parentVersion);
final String[] pVersion = parentVersion.split("\\.");

final int parentMajorVersion = Integer.parseInt(pVersion.get(0));
final int parentMinorVersion = Integer.parseInt(pVersion.get(1));
final int parentMajorVersion = Integer.parseInt(pVersion[0]);
final int parentMinorVersion = Integer.parseInt(pVersion[1]);

final String childName = properties.getProperty("org.apache.arrow.flight.jdbc-driver.name");
final String childVersion = properties.getProperty("org.apache.arrow.flight.jdbc-driver.version");
final List<String> cVersion = Splitter.on('.').splitToList(childVersion);
final String[] cVersion = childVersion.split("\\.");

final int childMajorVersion = Integer.parseInt(cVersion.get(0));
final int childMinorVersion = Integer.parseInt(cVersion.get(1));
final int childMajorVersion = Integer.parseInt(cVersion[0]);
final int childMinorVersion = Integer.parseInt(cVersion[1]);

version = new DriverVersion(
childName,
Expand Down Expand Up @@ -271,7 +269,7 @@ Optional<Map<Object, Object>> getUrlsArgs(String url)

static Properties lowerCasePropertyKeys(final Properties properties) {
final Properties resultProperty = new Properties();
properties.forEach((k, v) -> resultProperty.put(Ascii.toLowerCase(k.toString()), v));
properties.forEach((k, v) -> resultProperty.put(k.toString().toLowerCase(), v));
return resultProperty;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
import org.apache.calcite.avatica.ConnectionConfigImpl;
import org.apache.calcite.avatica.ConnectionProperty;

import com.google.common.base.Ascii;

/**
* A {@link ConnectionConfig} for the {@link ArrowFlightConnection}.
Expand Down Expand Up @@ -236,7 +235,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(Ascii.toLowerCase(camelName));
value = properties.get(camelName.toLowerCase());
}
if (required) {
if (value == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,10 @@
import java.net.URLDecoder;
import java.util.HashMap;
import java.util.Map;

import com.google.common.base.Splitter;

/**
* URL Parser for extracting key values from a connection string.
*/

public final class UrlParser {
private UrlParser() {
}
Expand All @@ -39,10 +37,11 @@ private UrlParser() {
* @param url {@link String}
* @return {@link Map}
*/
@SuppressWarnings("StringSplitter")
public static Map<String, String> parse(String url, String separator) {
Map<String, String> resultMap = new HashMap<>();
if (url != null) {
Iterable<String> keyValues = Splitter.onPattern(separator).split(url);
String[] keyValues = url.split(separator);

for (String keyValue : keyValues) {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,6 @@
import org.apache.commons.pool2.impl.GenericObjectPool;
import org.slf4j.Logger;

import com.google.common.base.Splitter;
import com.google.common.cache.Cache;
import com.google.common.cache.CacheBuilder;
import com.google.common.cache.RemovalListener;
Expand Down Expand Up @@ -384,6 +383,7 @@ private static <T extends FieldVector> int saveToVectors(final Map<T, String> ve
return saveToVectors(vectorToColumnName, data, emptyToNull, alwaysTrue);
}

@SuppressWarnings("StringSplitter")
private static <T extends FieldVector> int saveToVectors(final Map<T, String> vectorToColumnName,
final ResultSet data, boolean emptyToNull,
Predicate<ResultSet> resultSetPredicate)
Expand Down Expand Up @@ -424,11 +424,11 @@ private static <T extends FieldVector> int saveToVectors(final Map<T, String> ve
writer.startList();

if (createParamsValues != null) {
List<String> split = Splitter.on(',').splitToList(createParamsValues);
String[] split = createParamsValues.split(",");

range(0, split.size())
range(0, split.length)
.forEach(i -> {
byte[] bytes = split.get(i).getBytes(UTF_8);
byte[] bytes = split[i].getBytes(UTF_8);
Preconditions.checkState(bytes.length < 1024,
"The amount of bytes is greater than what the ArrowBuf supports");
buf.setBytes(0, bytes);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,6 @@ public VectorSchemaRoot slice(int index, int length) {
/**
* Determine if two VectorSchemaRoots are exactly equal.
*/

public boolean equals(VectorSchemaRoot other) {
if (other == null) {
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public abstract class AbstractStructVector extends AbstractContainerVector {
}
ConflictPolicy conflictPolicy;
try {
conflictPolicy = ConflictPolicy.valueOf(conflictPolicyStr.toUpperCase(Locale.getDefault()));
conflictPolicy = ConflictPolicy.valueOf(conflictPolicyStr.toUpperCase(Locale.ROOT));
} catch (Exception e) {
conflictPolicy = ConflictPolicy.CONFLICT_REPLACE;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ protected FieldWriter getWriter() {

private FieldWriter promoteToUnion() {
String name = vector.getField().getName();
TransferPair tp = vector.getTransferPair(vector.getMinorType().name().toLowerCase(Locale.getDefault()),
TransferPair tp = vector.getTransferPair(vector.getMinorType().name().toLowerCase(Locale.ROOT),
vector.getAllocator());
tp.transfer();
if (parentContainer != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,9 +202,7 @@ public int find(String what, int start) {
public void set(String string) {
try {
ByteBuffer bb = encode(string, true);
bytes = new byte[bb.remaining()];
bb.get(bytes);
bb.position(bb.position() - bytes.length); // move it back so we can decode it
bytes = bb.array();
length = bb.limit();
} catch (CharacterCodingException e) {
throw new RuntimeException("Should not have happened ", e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -221,44 +221,42 @@ public void testSetLastSetUsage() throws Exception {
offset = (int) offsetBuffer.getLong(index * LargeListVector.OFFSET_WIDTH);
assertEquals(Integer.toString(0), Integer.toString(offset));

Long actual = dataVector.getObject(offset);
assertEquals(Long.valueOf(10), actual);
long actual = dataVector.getObject(offset);
assertEquals(10L, actual);
offset++;
actual = dataVector.getObject(offset);
assertEquals(Long.valueOf(11), actual);
assertEquals(11L, actual);
offset++;
actual = dataVector.getObject(offset);
assertEquals(Long.valueOf(12), actual);
assertEquals(12L, actual);

index++;
offset = (int) offsetBuffer.getLong(index * LargeListVector.OFFSET_WIDTH);
assertEquals(Integer.toString(3), Integer.toString(offset));

actual = dataVector.getObject(offset);
assertEquals(Long.valueOf(13), actual);
assertEquals(13L, actual);
offset++;
actual = dataVector.getObject(offset);
assertEquals(Long.valueOf(14), actual);
assertEquals(14L, actual);

index++;
offset = (int) offsetBuffer.getLong(index * LargeListVector.OFFSET_WIDTH);
assertEquals(Integer.toString(5), Integer.toString(offset));

actual = dataVector.getObject(offset);
assertEquals(Long.valueOf(15), actual);
assertEquals(15L, actual);
offset++;
actual = dataVector.getObject(offset);
assertEquals(Long.valueOf(16), actual);
assertEquals(16L, actual);
offset++;
actual = dataVector.getObject(offset);
assertEquals(Long.valueOf(17), actual);
assertEquals(17L, actual);

index++;
offset = (int) offsetBuffer.getLong(index * LargeListVector.OFFSET_WIDTH);
assertEquals(Integer.toString(8), Integer.toString(offset));

actual = dataVector.getObject(offset);
assertNull(actual);
assertNull(dataVector.getObject(offset));
}
}

Expand Down

0 comments on commit 220c6d4

Please sign in to comment.