You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by jh...@apache.org on 2015/10/26 23:36:18 UTC
[5/6] incubator-calcite git commit: [CALCITE-921] Fix incorrectness
when calling getString() on binary data (Josh Elser)
[CALCITE-921] Fix incorrectness when calling getString() on binary data (Josh Elser)
Close apache/incubator-calcite#160
Project: http://git-wip-us.apache.org/repos/asf/incubator-calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-calcite/commit/6f326d03
Tree: http://git-wip-us.apache.org/repos/asf/incubator-calcite/tree/6f326d03
Diff: http://git-wip-us.apache.org/repos/asf/incubator-calcite/diff/6f326d03
Branch: refs/heads/master
Commit: 6f326d0391923f4c4af5f9e60eb2ce263fdad081
Parents: bf178d5
Author: Josh Elser <el...@apache.org>
Authored: Tue Oct 13 19:31:47 2015 -0400
Committer: Julian Hyde <jh...@apache.org>
Committed: Mon Oct 26 12:13:43 2015 -0700
----------------------------------------------------------------------
.../calcite/avatica/remote/RemoteMetaTest.java | 31 +++++++++++++++++
.../calcite/avatica/remote/AbstractService.java | 22 +++++++++++-
.../calcite/avatica/remote/JsonService.java | 4 +++
.../calcite/avatica/remote/ProtobufService.java | 5 +++
.../calcite/avatica/util/AbstractCursor.java | 35 ++++++++++++++++++--
5 files changed, 93 insertions(+), 4 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-calcite/blob/6f326d03/avatica-server/src/test/java/org/apache/calcite/avatica/remote/RemoteMetaTest.java
----------------------------------------------------------------------
diff --git a/avatica-server/src/test/java/org/apache/calcite/avatica/remote/RemoteMetaTest.java b/avatica-server/src/test/java/org/apache/calcite/avatica/remote/RemoteMetaTest.java
index 3ccebf8..e4d8690 100644
--- a/avatica-server/src/test/java/org/apache/calcite/avatica/remote/RemoteMetaTest.java
+++ b/avatica-server/src/test/java/org/apache/calcite/avatica/remote/RemoteMetaTest.java
@@ -42,13 +42,16 @@ import org.junit.runners.Parameterized.Parameters;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
+import java.nio.charset.StandardCharsets;
import java.sql.Array;
import java.sql.Connection;
import java.sql.DriverManager;
+import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Random;
@@ -416,6 +419,34 @@ public class RemoteMetaTest {
ConnectionSpec.getDatabaseLock().unlock();
}
}
+
+ @Test public void testBinaryAndStrings() throws Exception {
+ final String tableName = "testbinaryandstrs";
+ final byte[] data = "asdf".getBytes(StandardCharsets.UTF_8);
+ ConnectionSpec.getDatabaseLock().lock();
+ try (final Connection conn = DriverManager.getConnection(url);
+ final Statement stmt = conn.createStatement()) {
+ assertFalse(stmt.execute("DROP TABLE IF EXISTS " + tableName));
+ assertFalse(stmt.execute("CREATE TABLE " + tableName + "(id int, bin BINARY(4))"));
+ try (final PreparedStatement prepStmt = conn.prepareStatement(
+ "INSERT INTO " + tableName + " values(1, ?)")) {
+ prepStmt.setBytes(1, data);
+ assertFalse(prepStmt.execute());
+ }
+ try (ResultSet results = stmt.executeQuery("SELECT id, bin from " + tableName)) {
+ assertTrue(results.next());
+ assertEquals(1, results.getInt(1));
+ // byte comparison should work
+ assertArrayEquals("Bytes were " + Arrays.toString(results.getBytes(2)),
+ data, results.getBytes(2));
+ // as should string
+ assertEquals(new String(data, StandardCharsets.UTF_8), results.getString(2));
+ assertFalse(results.next());
+ }
+ } finally {
+ ConnectionSpec.getDatabaseLock().unlock();
+ }
+ }
}
// End RemoteMetaTest.java
http://git-wip-us.apache.org/repos/asf/incubator-calcite/blob/6f326d03/avatica/src/main/java/org/apache/calcite/avatica/remote/AbstractService.java
----------------------------------------------------------------------
diff --git a/avatica/src/main/java/org/apache/calcite/avatica/remote/AbstractService.java b/avatica/src/main/java/org/apache/calcite/avatica/remote/AbstractService.java
index e88a47c..93ad3ef 100644
--- a/avatica/src/main/java/org/apache/calcite/avatica/remote/AbstractService.java
+++ b/avatica/src/main/java/org/apache/calcite/avatica/remote/AbstractService.java
@@ -29,6 +29,19 @@ import java.util.List;
*/
public abstract class AbstractService implements Service {
+ /**
+ * Represents the serialization of the data over a transport.
+ */
+ enum SerializationType {
+ JSON,
+ PROTOBUF
+ }
+
+ /**
+ * @return The manner in which the data is serialized.
+ */
+ abstract SerializationType getSerializationType();
+
/** Modifies a signature, changing the representation of numeric columns
* within it. This deals with the fact that JSON transmits a small long value,
* or a float which is a whole number, as an integer. Thus the accessors need
@@ -68,7 +81,14 @@ public abstract class AbstractService implements Service {
switch (column.type.id) {
case Types.VARBINARY:
case Types.BINARY:
- return column.setRep(ColumnMetaData.Rep.STRING);
+ switch (getSerializationType()) {
+ case JSON:
+ return column.setRep(ColumnMetaData.Rep.STRING);
+ case PROTOBUF:
+ return column;
+ default:
+ throw new IllegalStateException("Unhadled case statement");
+ }
case Types.DECIMAL:
case Types.NUMERIC:
return column.setRep(ColumnMetaData.Rep.NUMBER);
http://git-wip-us.apache.org/repos/asf/incubator-calcite/blob/6f326d03/avatica/src/main/java/org/apache/calcite/avatica/remote/JsonService.java
----------------------------------------------------------------------
diff --git a/avatica/src/main/java/org/apache/calcite/avatica/remote/JsonService.java b/avatica/src/main/java/org/apache/calcite/avatica/remote/JsonService.java
index 86d69f1..a223069 100644
--- a/avatica/src/main/java/org/apache/calcite/avatica/remote/JsonService.java
+++ b/avatica/src/main/java/org/apache/calcite/avatica/remote/JsonService.java
@@ -43,6 +43,10 @@ public abstract class JsonService extends AbstractService {
* responses to and from the peer service. */
public abstract String apply(String request);
+ @Override SerializationType getSerializationType() {
+ return SerializationType.JSON;
+ }
+
//@VisibleForTesting
protected static <T> T decode(String response, Class<T> expectedType)
throws IOException {
http://git-wip-us.apache.org/repos/asf/incubator-calcite/blob/6f326d03/avatica/src/main/java/org/apache/calcite/avatica/remote/ProtobufService.java
----------------------------------------------------------------------
diff --git a/avatica/src/main/java/org/apache/calcite/avatica/remote/ProtobufService.java b/avatica/src/main/java/org/apache/calcite/avatica/remote/ProtobufService.java
index 35cb35a..8414708 100644
--- a/avatica/src/main/java/org/apache/calcite/avatica/remote/ProtobufService.java
+++ b/avatica/src/main/java/org/apache/calcite/avatica/remote/ProtobufService.java
@@ -17,6 +17,7 @@
package org.apache.calcite.avatica.remote;
import com.google.protobuf.Descriptors.Descriptor;
+
import com.google.protobuf.Message;
/**
@@ -30,6 +31,10 @@ public abstract class ProtobufService extends AbstractService {
*/
public abstract Response _apply(Request request);
+ @Override SerializationType getSerializationType() {
+ return SerializationType.PROTOBUF;
+ }
+
@Override public ResultSetResponse apply(CatalogsRequest request) {
return finagle((ResultSetResponse) _apply(request));
}
http://git-wip-us.apache.org/repos/asf/incubator-calcite/blob/6f326d03/avatica/src/main/java/org/apache/calcite/avatica/util/AbstractCursor.java
----------------------------------------------------------------------
diff --git a/avatica/src/main/java/org/apache/calcite/avatica/util/AbstractCursor.java b/avatica/src/main/java/org/apache/calcite/avatica/util/AbstractCursor.java
index cd65199..dd38da0 100644
--- a/avatica/src/main/java/org/apache/calcite/avatica/util/AbstractCursor.java
+++ b/avatica/src/main/java/org/apache/calcite/avatica/util/AbstractCursor.java
@@ -26,6 +26,7 @@ import java.lang.reflect.Field;
import java.math.BigDecimal;
import java.math.RoundingMode;
import java.net.URL;
+import java.nio.charset.StandardCharsets;
import java.sql.Array;
import java.sql.Blob;
import java.sql.Clob;
@@ -782,7 +783,7 @@ public abstract class AbstractCursor implements Cursor {
}
//FIXME: Protobuf gets byte[]
- public byte[] getBytes() {
+ @Override public byte[] getBytes() {
Object obj = getObject();
try {
final ByteString o = (ByteString) obj;
@@ -791,6 +792,19 @@ public abstract class AbstractCursor implements Cursor {
return obj == null ? null : (byte[]) obj;
}
}
+
+ @Override public String getString() {
+ Object o = getObject();
+ if (null == o) {
+ return null;
+ }
+ if (o instanceof byte[]) {
+ return new String((byte[]) o, StandardCharsets.UTF_8);
+ } else if (o instanceof ByteString) {
+ return ((ByteString) o).toString();
+ }
+ throw new IllegalStateException("Unhandled value type: " + o.getClass());
+ }
}
/**
@@ -810,17 +824,32 @@ public abstract class AbstractCursor implements Cursor {
@Override public byte[] getBytes() {
// JSON sends this as a base64-enc string, protobuf can do binary.
Object obj = getObject();
+
if (obj instanceof byte[]) {
// If we already have bytes, just send them back.
return (byte[]) obj;
}
- final String string = getString();
- if (string == null) {
+ return getBase64Decoded();
+ }
+
+ private byte[] getBase64Decoded() {
+ final String string = super.getString();
+ if (null == string) {
return null;
}
+ // Need to base64 decode the string.
return ByteString.parseBase64(string);
}
+
+ @Override public String getString() {
+ final byte[] bytes = getBase64Decoded();
+ if (null == bytes) {
+ return null;
+ }
+ // Need to base64 decode the string.
+ return new String(bytes, StandardCharsets.UTF_8);
+ }
}
/**