Repository: calcite Updated Branches: refs/heads/master 3e25f2ffa -> aa9db8a36
[CALCITE-1103] Correct decimal serialization with protobuf Decimal values were incorrectly being treated as generic "numbers" which were causing them to be serialized as a "double" value. This lost some of the extra details on the decimal value which caused incorrect results. Meta.Frame was also reworked to use TypedValue for serialization into protobuf. This, combined with a simplification of TypedValue's serialization methods, results in an overall reduction of translation code to/from protobufs. Also includes a test to verify that we can still parse Decimals (although, truncated) via the former method. Project: http://git-wip-us.apache.org/repos/asf/calcite/repo Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/aa9db8a3 Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/aa9db8a3 Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/aa9db8a3 Branch: refs/heads/master Commit: aa9db8a36046b1e812021fa1cf0973906cfffbad Parents: 3e25f2f Author: Josh Elser <[email protected]> Authored: Wed Mar 30 16:56:26 2016 -0400 Committer: Josh Elser <[email protected]> Committed: Mon Apr 4 11:10:39 2016 -0400 ---------------------------------------------------------------------- .../apache/calcite/avatica/ColumnMetaData.java | 6 + .../java/org/apache/calcite/avatica/Meta.java | 87 ++----- .../calcite/avatica/remote/TypedValue.java | 235 +++++++++++-------- .../calcite/avatica/util/AbstractCursor.java | 16 +- .../calcite/avatica/remote/TypedValueTest.java | 101 +++++--- .../calcite/avatica/RemoteDriverTest.java | 25 ++ 6 files changed, 261 insertions(+), 209 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/calcite/blob/aa9db8a3/avatica/core/src/main/java/org/apache/calcite/avatica/ColumnMetaData.java ---------------------------------------------------------------------- diff --git a/avatica/core/src/main/java/org/apache/calcite/avatica/ColumnMetaData.java b/avatica/core/src/main/java/org/apache/calcite/avatica/ColumnMetaData.java index bcdc228..401070e 100644 --- a/avatica/core/src/main/java/org/apache/calcite/avatica/ColumnMetaData.java +++ b/avatica/core/src/main/java/org/apache/calcite/avatica/ColumnMetaData.java @@ -429,6 +429,12 @@ public class ColumnMetaData { } public static Rep fromProto(Common.Rep proto) { + if (Common.Rep.BIG_DECIMAL == proto) { + // BIG_DECIMAL has to come back as a NUMBER + return Rep.NUMBER; + } else if (Common.Rep.NULL == proto) { + return Rep.OBJECT; + } return Rep.valueOf(proto.name()); } } http://git-wip-us.apache.org/repos/asf/calcite/blob/aa9db8a3/avatica/core/src/main/java/org/apache/calcite/avatica/Meta.java ---------------------------------------------------------------------- diff --git a/avatica/core/src/main/java/org/apache/calcite/avatica/Meta.java b/avatica/core/src/main/java/org/apache/calcite/avatica/Meta.java index 80f384f..4fd2b17 100644 --- a/avatica/core/src/main/java/org/apache/calcite/avatica/Meta.java +++ b/avatica/core/src/main/java/org/apache/calcite/avatica/Meta.java @@ -25,12 +25,10 @@ import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonSubTypes; import com.fasterxml.jackson.annotation.JsonTypeInfo; -import com.google.protobuf.ByteString; import com.google.protobuf.Descriptors.FieldDescriptor; import java.lang.reflect.Field; import java.lang.reflect.Method; -import java.math.BigDecimal; import java.sql.Connection; import java.sql.DatabaseMetaData; import java.sql.ResultSet; @@ -930,43 +928,8 @@ public interface Meta { static Common.TypedValue serializeScalar(Object element) { final Common.TypedValue.Builder valueBuilder = Common.TypedValue.newBuilder(); - // Numbers - if (element instanceof Byte) { - valueBuilder.setType(Common.Rep.BYTE).setNumberValue(((Byte) element).longValue()); - } else if (element instanceof Short) { - valueBuilder.setType(Common.Rep.SHORT).setNumberValue(((Short) element).longValue()); - } else if (element instanceof Integer) { - valueBuilder.setType(Common.Rep.INTEGER) - .setNumberValue(((Integer) element).longValue()); - } else if (element instanceof Long) { - valueBuilder.setType(Common.Rep.LONG).setNumberValue((Long) element); - } else if (element instanceof Double) { - valueBuilder.setType(Common.Rep.DOUBLE).setDoubleValue((Double) element); - } else if (element instanceof Float) { - valueBuilder.setType(Common.Rep.FLOAT).setNumberValue(((Float) element).longValue()); - } else if (element instanceof BigDecimal) { - valueBuilder.setType(Common.Rep.NUMBER) - .setDoubleValue(((BigDecimal) element).doubleValue()); - // Strings - } else if (element instanceof String) { - valueBuilder.setType(Common.Rep.STRING) - .setStringValue((String) element); - } else if (element instanceof Character) { - valueBuilder.setType(Common.Rep.CHARACTER) - .setStringValue(element.toString()); - // Bytes - } else if (element instanceof byte[]) { - valueBuilder.setType(Common.Rep.BYTE_STRING) - .setBytesValues(ByteString.copyFrom((byte[]) element)); - // Boolean - } else if (element instanceof Boolean) { - valueBuilder.setType(Common.Rep.BOOLEAN).setBoolValue((boolean) element); - } else if (null == element) { - valueBuilder.setType(Common.Rep.NULL); - // Unhandled - } else { - throw new RuntimeException("Unhandled type in Frame: " + element.getClass()); - } + // Let TypedValue handle the serialization for us. + TypedValue.toProto(valueBuilder, element); return valueBuilder.build(); } @@ -1019,11 +982,11 @@ public interface Meta { if (column.getValueCount() > 1) { List<Object> array = new ArrayList<>(column.getValueCount()); for (Common.TypedValue columnValue : column.getValueList()) { - array.add(getScalarValue(columnValue)); + array.add(deserializeScalarValue(columnValue)); } return array; } else { - return getScalarValue(column.getValue(0)); + return deserializeScalarValue(column.getValue(0)); } } @@ -1041,12 +1004,12 @@ public interface Meta { // Array List<Object> array = new ArrayList<>(column.getArrayValueCount()); for (Common.TypedValue arrayValue : column.getArrayValueList()) { - array.add(getScalarValue(arrayValue)); + array.add(deserializeScalarValue(arrayValue)); } return array; } else { // Scalar - return getScalarValue(column.getScalarValue()); + return deserializeScalarValue(column.getScalarValue()); } } @@ -1067,38 +1030,16 @@ public interface Meta { } } - static Object getScalarValue(Common.TypedValue protoElement) { - // TODO Should these be primitives or Objects? - switch (protoElement.getType()) { - case BYTE: - return Long.valueOf(protoElement.getNumberValue()).byteValue(); - case SHORT: - return Long.valueOf(protoElement.getNumberValue()).shortValue(); - case INTEGER: - return Long.valueOf(protoElement.getNumberValue()).intValue(); - case LONG: - return protoElement.getNumberValue(); - case FLOAT: - return Long.valueOf(protoElement.getNumberValue()).floatValue(); - case DOUBLE: - return protoElement.getDoubleValue(); - case NUMBER: - // TODO more cases here to expand on? BigInteger? - return BigDecimal.valueOf(protoElement.getDoubleValue()); - case STRING: - return protoElement.getStringValue(); - case CHARACTER: - // A single character in the string - return protoElement.getStringValue().charAt(0); - case BYTE_STRING: + static Object deserializeScalarValue(Common.TypedValue protoElement) { + // ByteString is a single case where TypedValue is representing the data differently + // (in its "local" form) than Frame does. We need to unwrap the Base64 encoding. + if (Common.Rep.BYTE_STRING == protoElement.getType()) { + // Protobuf is sending native bytes (not b64) across the wire. B64 bytes is only for + // TypedValue's benefit return protoElement.getBytesValues().toByteArray(); - case BOOLEAN: - return protoElement.getBoolValue(); - case NULL: - return null; - default: - throw new RuntimeException("Unhandled type: " + protoElement.getType()); } + // Again, let TypedValue deserialize things for us. + return TypedValue.fromProto(protoElement).value; } @Override public int hashCode() { http://git-wip-us.apache.org/repos/asf/calcite/blob/aa9db8a3/avatica/core/src/main/java/org/apache/calcite/avatica/remote/TypedValue.java ---------------------------------------------------------------------- diff --git a/avatica/core/src/main/java/org/apache/calcite/avatica/remote/TypedValue.java b/avatica/core/src/main/java/org/apache/calcite/avatica/remote/TypedValue.java index 1146a47..bbf440c 100644 --- a/avatica/core/src/main/java/org/apache/calcite/avatica/remote/TypedValue.java +++ b/avatica/core/src/main/java/org/apache/calcite/avatica/remote/TypedValue.java @@ -17,12 +17,14 @@ package org.apache.calcite.avatica.remote; import org.apache.calcite.avatica.ColumnMetaData; +import org.apache.calcite.avatica.ColumnMetaData.Rep; import org.apache.calcite.avatica.proto.Common; import org.apache.calcite.avatica.util.ByteString; import org.apache.calcite.avatica.util.DateTimeUtils; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.protobuf.Descriptors.FieldDescriptor; import com.google.protobuf.HBaseZeroCopyByteString; import java.math.BigDecimal; @@ -120,6 +122,9 @@ import java.util.Objects; * </ul> */ public class TypedValue { + private static final FieldDescriptor NUMBER_DESCRIPTOR = Common.TypedValue.getDescriptor() + .findFieldByNumber(Common.TypedValue.NUMBER_VALUE_FIELD_NUMBER); + public static final TypedValue NULL = new TypedValue(ColumnMetaData.Rep.OBJECT, null); @@ -245,40 +250,6 @@ public class TypedValue { } } - private static Object protoSerialToLocal(Common.Rep rep, Object value) { - switch (rep) { - case BYTE: - return ((Number) value).byteValue(); - case SHORT: - return ((Number) value).shortValue(); - case INTEGER: - case JAVA_SQL_DATE: - case JAVA_SQL_TIME: - return ((Number) value).intValue(); - case LONG: - case JAVA_UTIL_DATE: - case JAVA_SQL_TIMESTAMP: - return ((Number) value).longValue(); - case FLOAT: - return ((Number) value).floatValue(); - case DOUBLE: - return ((Number) value).doubleValue(); - case NUMBER: - return value instanceof BigDecimal ? value - : value instanceof BigInteger ? new BigDecimal((BigInteger) value) - : value instanceof Double ? new BigDecimal((Double) value) - : value instanceof Float ? new BigDecimal((Float) value) - : new BigDecimal(((Number) value).longValue()); - case BYTE_STRING: - return (byte[]) value; - case STRING: - return (String) value; - default: - throw new IllegalArgumentException("cannot convert " + value + " (" - + value.getClass() + ") to " + rep); - } - } - /** Converts the value into the JDBC representation. * * <p>For example, a byte string is represented as a {@link ByteString}; @@ -291,8 +262,15 @@ public class TypedValue { return serialToJdbc(type, value, calendar); } - private static Object serialToJdbc(ColumnMetaData.Rep type, Object value, - Calendar calendar) { + /** + * Converts the given value from serial form to JDBC form. + * + * @param type The type of the value + * @param value The value + * @param calendar A calendar instance + * @return The JDBC representation of the value. + */ + private static Object serialToJdbc(ColumnMetaData.Rep type, Object value, Calendar calendar) { switch (type) { case BYTE_STRING: return ByteString.ofBase64((String) value).getBytes(); @@ -311,22 +289,6 @@ public class TypedValue { } } - private static Object protoSerialToJdbc(Common.Rep type, Object value, Calendar calendar) { - switch (type) { - case JAVA_UTIL_DATE: - return new java.util.Date(adjust((Number) value, calendar)); - case JAVA_SQL_DATE: - return new java.sql.Date( - adjust(((Number) value).longValue() * DateTimeUtils.MILLIS_PER_DAY, calendar)); - case JAVA_SQL_TIME: - return new java.sql.Time(adjust((Number) value, calendar)); - case JAVA_SQL_TIMESTAMP: - return new java.sql.Timestamp(adjust((Number) value, calendar)); - default: - return protoSerialToLocal(type, value); - } - } - private static long adjust(Number number, Calendar calendar) { long t = number.longValue(); if (calendar != null) { @@ -391,87 +353,107 @@ public class TypedValue { final Common.TypedValue.Builder builder = Common.TypedValue.newBuilder(); Common.Rep protoRep = type.toProto(); - builder.setType(protoRep); + // Protobuf has an explicit BIG_DECIMAL representation enum value. + if (Common.Rep.NUMBER == protoRep && value instanceof BigDecimal) { + protoRep = Common.Rep.BIG_DECIMAL; + } // Serialize the type into the protobuf - switch (protoRep) { + writeToProtoWithType(builder, value, protoRep); + + return builder.build(); + } + + private static void writeToProtoWithType(Common.TypedValue.Builder builder, Object o, + Common.Rep type) { + builder.setType(type); + + switch (type) { case BOOLEAN: case PRIMITIVE_BOOLEAN: - builder.setBoolValue((boolean) value); - break; + builder.setBoolValue((boolean) o); + return; case BYTE_STRING: + byte[] bytes; + // Serial representation is b64. We don't need to do that for protobuf + if (o instanceof String) { + // Assume strings are already b64 encoded + bytes = ByteString.parseBase64((String) o); + } else { + bytes = (byte[]) o; + } + builder.setBytesValues(HBaseZeroCopyByteString.wrap(bytes)); + return; case STRING: - builder.setStringValueBytes(HBaseZeroCopyByteString.wrap(((String) value).getBytes())); - break; + builder.setStringValueBytes(HBaseZeroCopyByteString.wrap(((String) o).getBytes())); + return; case PRIMITIVE_CHAR: case CHARACTER: - builder.setStringValue(Character.toString((char) value)); - break; + builder.setStringValue(Character.toString((char) o)); + return; case BYTE: case PRIMITIVE_BYTE: - builder.setNumberValue(Byte.valueOf((byte) value).longValue()); - break; + builder.setNumberValue(Byte.valueOf((byte) o).longValue()); + return; case DOUBLE: case PRIMITIVE_DOUBLE: - builder.setDoubleValue((double) value); - break; + builder.setDoubleValue((double) o); + return; case FLOAT: case PRIMITIVE_FLOAT: - builder.setNumberValue(Float.floatToIntBits((float) value)); - break; + builder.setNumberValue(Float.floatToIntBits((float) o)); + return; case INTEGER: case PRIMITIVE_INT: - builder.setNumberValue(Integer.valueOf((int) value).longValue()); - break; + builder.setNumberValue(Integer.valueOf((int) o).longValue()); + return; case PRIMITIVE_SHORT: case SHORT: - builder.setNumberValue(Short.valueOf((short) value).longValue()); - break; + builder.setNumberValue(Short.valueOf((short) o).longValue()); + return; case LONG: case PRIMITIVE_LONG: - builder.setNumberValue((long) value); - break; + builder.setNumberValue((long) o); + return; case JAVA_SQL_DATE: case JAVA_SQL_TIME: // Persisted as integers - builder.setNumberValue(Integer.valueOf((int) value).longValue()); - break; + builder.setNumberValue(Integer.valueOf((int) o).longValue()); + return; case JAVA_SQL_TIMESTAMP: case JAVA_UTIL_DATE: // Persisted as longs - builder.setNumberValue((long) value); - break; + builder.setNumberValue((long) o); + return; case BIG_INTEGER: - byte[] bytes = ((BigInteger) value).toByteArray(); - builder.setBytesValues(com.google.protobuf.ByteString.copyFrom(bytes)); - break; + byte[] byteRep = ((BigInteger) o).toByteArray(); + builder.setBytesValues(com.google.protobuf.ByteString.copyFrom(byteRep)); + return; case BIG_DECIMAL: - final BigDecimal bigDecimal = (BigDecimal) value; - final int scale = bigDecimal.scale(); - final BigInteger bigInt = bigDecimal.toBigInteger(); - builder.setBytesValues(com.google.protobuf.ByteString.copyFrom(bigInt.toByteArray())) - .setNumberValue(scale); - break; + final BigDecimal bigDecimal = (BigDecimal) o; + builder.setStringValue(bigDecimal.toString()); + return; case NUMBER: - builder.setNumberValue(((Number) value).longValue()); - break; + builder.setNumberValue(((Number) o).longValue()); + return; + case NULL: + builder.setNull(true); + return; case OBJECT: - if (null == value) { + if (null == o) { // We can persist a null value through easily builder.setNull(true); - break; + return; } // Intentional fall-through to RTE because we can't serialize something we have no type // insight into. case UNRECOGNIZED: // Fail? - throw new RuntimeException("Unhandled value: " + protoRep + " " + value.getClass()); + throw new RuntimeException("Unhandled value: " + type + " " + o.getClass()); default: // Fail? - throw new RuntimeException("Unknown serialized type: " + protoRep); + throw new RuntimeException("Unknown serialized type: " + type); } - - return builder.build(); } /** @@ -482,7 +464,7 @@ public class TypedValue { */ public static TypedValue fromProto(Common.TypedValue proto) { ColumnMetaData.Rep rep = ColumnMetaData.Rep.fromProto(proto.getType()); - Object value = getValue(proto); + Object value = getSerialFromProto(proto); return new TypedValue(rep, value); } @@ -493,16 +475,17 @@ public class TypedValue { * @param protoValue The serialized TypedValue. * @return The appropriate concrete type for the parameter value (as an Object). */ - public static Object getValue(Common.TypedValue protoValue) { + public static Object getSerialFromProto(Common.TypedValue protoValue) { // Deserialize the value again switch (protoValue.getType()) { case BOOLEAN: case PRIMITIVE_BOOLEAN: return protoValue.getBoolValue(); case BYTE_STRING: + // TypedValue is still going to expect a b64string for BYTE_STRING even though we sent it + // across the wire natively as bytes. Return it as b64. + return (new ByteString(protoValue.getBytesValues().toByteArray())).toBase64String(); case STRING: - // TypedValue is still going to expect a string for BYTE_STRING even though we sent it - // across the wire natively as bytes. return protoValue.getStringValue(); case PRIMITIVE_CHAR: case CHARACTER: @@ -534,10 +517,17 @@ public class TypedValue { case BIG_INTEGER: return new BigInteger(protoValue.getBytesValues().toByteArray()); case BIG_DECIMAL: - BigInteger bigInt = new BigInteger(protoValue.getBytesValues().toByteArray()); - return new BigDecimal(bigInt, (int) protoValue.getNumberValue()); + // CALCITE-1103 shifts BigDecimals to be serialized as strings. + if (protoValue.hasField(NUMBER_DESCRIPTOR)) { + // This is the old (broken) style. + BigInteger bigInt = new BigInteger(protoValue.getBytesValues().toByteArray()); + return new BigDecimal(bigInt, (int) protoValue.getNumberValue()); + } + return new BigDecimal(protoValue.getStringValueBytes().toStringUtf8()); case NUMBER: return Long.valueOf(protoValue.getNumberValue()); + case NULL: + return null; case OBJECT: if (protoValue.getNull()) { return null; @@ -554,6 +544,48 @@ public class TypedValue { } /** + * Writes the given object into the Protobuf representation of a TypedValue. The object is + * serialized given the type of that object, mapping it to the appropriate representation. + * + * @param builder The TypedValue protobuf builder + * @param o The object (value) + */ + public static void toProto(Common.TypedValue.Builder builder, Object o) { + // Numbers + if (o instanceof Byte) { + writeToProtoWithType(builder, o, Common.Rep.BYTE); + } else if (o instanceof Short) { + writeToProtoWithType(builder, o, Common.Rep.SHORT); + } else if (o instanceof Integer) { + writeToProtoWithType(builder, o, Common.Rep.INTEGER); + } else if (o instanceof Long) { + writeToProtoWithType(builder, o, Common.Rep.LONG); + } else if (o instanceof Double) { + writeToProtoWithType(builder, o, Common.Rep.DOUBLE); + } else if (o instanceof Float) { + writeToProtoWithType(builder, ((Float) o).longValue(), Common.Rep.FLOAT); + } else if (o instanceof BigDecimal) { + writeToProtoWithType(builder, o, Common.Rep.BIG_DECIMAL); + // Strings + } else if (o instanceof String) { + writeToProtoWithType(builder, o, Common.Rep.STRING); + } else if (o instanceof Character) { + writeToProtoWithType(builder, o.toString(), Common.Rep.CHARACTER); + // Bytes + } else if (o instanceof byte[]) { + writeToProtoWithType(builder, o, Common.Rep.BYTE_STRING); + // Boolean + } else if (o instanceof Boolean) { + writeToProtoWithType(builder, o, Common.Rep.BOOLEAN); + } else if (null == o) { + writeToProtoWithType(builder, o, Common.Rep.NULL); + // Unhandled + } else { + throw new RuntimeException("Unhandled type in Frame: " + o.getClass()); + } + } + + /** * Extracts the JDBC value from protobuf-TypedValue representation. * * @param protoValue Protobuf TypedValue @@ -561,12 +593,13 @@ public class TypedValue { * @return The JDBC representation of this TypedValue */ public static Object protoToJdbc(Common.TypedValue protoValue, Calendar calendar) { - Object o = getValue(Objects.requireNonNull(protoValue)); + Object o = getSerialFromProto(Objects.requireNonNull(protoValue)); // Shortcircuit the null if (null == o) { return o; } - return protoSerialToJdbc(protoValue.getType(), o, Objects.requireNonNull(calendar)); + return serialToJdbc(Rep.fromProto(protoValue.getType()), o, calendar); + //return protoSerialToJdbc(protoValue.getType(), o, Objects.requireNonNull(calendar)); } @Override public int hashCode() { http://git-wip-us.apache.org/repos/asf/calcite/blob/aa9db8a3/avatica/core/src/main/java/org/apache/calcite/avatica/util/AbstractCursor.java ---------------------------------------------------------------------- diff --git a/avatica/core/src/main/java/org/apache/calcite/avatica/util/AbstractCursor.java b/avatica/core/src/main/java/org/apache/calcite/avatica/util/AbstractCursor.java index 70f87a7..3b46b6c 100644 --- a/avatica/core/src/main/java/org/apache/calcite/avatica/util/AbstractCursor.java +++ b/avatica/core/src/main/java/org/apache/calcite/avatica/util/AbstractCursor.java @@ -785,11 +785,17 @@ public abstract class AbstractCursor implements Cursor { //FIXME: Protobuf gets byte[] @Override public byte[] getBytes() { Object obj = getObject(); - try { - final ByteString o = (ByteString) obj; - return o == null ? null : o.getBytes(); - } catch (Exception ex) { - return obj == null ? null : (byte[]) obj; + if (null == obj) { + return null; + } + if (obj instanceof ByteString) { + return ((ByteString) obj).getBytes(); + } else if (obj instanceof String) { + return ((String) obj).getBytes(); + } else if (obj instanceof byte[]) { + return (byte[]) obj; + } else { + throw new RuntimeException("Cannot handle " + obj.getClass() + " as bytes"); } } http://git-wip-us.apache.org/repos/asf/calcite/blob/aa9db8a3/avatica/core/src/test/java/org/apache/calcite/avatica/remote/TypedValueTest.java ---------------------------------------------------------------------- diff --git a/avatica/core/src/test/java/org/apache/calcite/avatica/remote/TypedValueTest.java b/avatica/core/src/test/java/org/apache/calcite/avatica/remote/TypedValueTest.java index 28fe6f6..5eed007 100644 --- a/avatica/core/src/test/java/org/apache/calcite/avatica/remote/TypedValueTest.java +++ b/avatica/core/src/test/java/org/apache/calcite/avatica/remote/TypedValueTest.java @@ -17,13 +17,23 @@ package org.apache.calcite.avatica.remote; import org.apache.calcite.avatica.ColumnMetaData.Rep; +import org.apache.calcite.avatica.proto.Common; import org.apache.calcite.avatica.util.ByteString; import org.junit.Test; -import java.nio.charset.StandardCharsets; +import java.math.BigDecimal; +import java.util.Calendar; +import java.util.GregorianCalendar; +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThat; + +import static java.nio.charset.StandardCharsets.UTF_8; /** * Test serialization of TypedValue. @@ -37,90 +47,121 @@ public class TypedValueTest { assertEquals(value.value, copy.value); } - @Test - public void testBoolean() { + @Test public void testBoolean() { serializeAndEqualityCheck(TypedValue.ofLocal(Rep.PRIMITIVE_BOOLEAN, true)); serializeAndEqualityCheck(TypedValue.ofLocal(Rep.BOOLEAN, Boolean.TRUE)); } - @Test - public void testByte() { + @Test public void testByte() { serializeAndEqualityCheck(TypedValue.ofLocal(Rep.PRIMITIVE_BYTE, (byte) 4)); serializeAndEqualityCheck(TypedValue.ofLocal(Rep.BYTE, Byte.valueOf((byte) 4))); } - @Test - public void testShort() { + @Test public void testShort() { serializeAndEqualityCheck(TypedValue.ofLocal(Rep.PRIMITIVE_SHORT, (short) 42)); serializeAndEqualityCheck(TypedValue.ofLocal(Rep.SHORT, Short.valueOf((short) 42))); } - @Test - public void testInteger() { + @Test public void testInteger() { serializeAndEqualityCheck(TypedValue.ofLocal(Rep.PRIMITIVE_INT, (int) 42000)); serializeAndEqualityCheck(TypedValue.ofLocal(Rep.INTEGER, Integer.valueOf((int) 42000))); } - @Test - public void testLong() { + @Test public void testLong() { serializeAndEqualityCheck(TypedValue.ofLocal(Rep.PRIMITIVE_LONG, Long.MAX_VALUE)); serializeAndEqualityCheck(TypedValue.ofLocal(Rep.LONG, Long.valueOf(Long.MAX_VALUE))); } - @Test - public void testFloat() { + @Test public void testFloat() { serializeAndEqualityCheck(TypedValue.ofLocal(Rep.PRIMITIVE_FLOAT, 3.14159f)); serializeAndEqualityCheck(TypedValue.ofLocal(Rep.FLOAT, Float.valueOf(3.14159f))); } - @Test - public void testDouble() { + @Test public void testDouble() { serializeAndEqualityCheck(TypedValue.ofLocal(Rep.PRIMITIVE_DOUBLE, Double.MAX_VALUE)); serializeAndEqualityCheck(TypedValue.ofLocal(Rep.DOUBLE, Double.valueOf(Double.MAX_VALUE))); } - @Test - public void testChar() { + @Test public void testDecimal() { + final BigDecimal decimal = new BigDecimal("1.2345"); + final TypedValue decimalTypedValue = TypedValue.ofLocal(Rep.NUMBER, decimal); + serializeAndEqualityCheck(decimalTypedValue); + + final Common.TypedValue protoTypedValue = decimalTypedValue.toProto(); + assertEquals(Common.Rep.BIG_DECIMAL, protoTypedValue.getType()); + final String strValue = protoTypedValue.getStringValue(); + assertNotNull(strValue); + assertEquals(decimal.toPlainString(), strValue); + } + + @Test public void testChar() { serializeAndEqualityCheck(TypedValue.ofLocal(Rep.PRIMITIVE_CHAR, 'c')); serializeAndEqualityCheck(TypedValue.ofLocal(Rep.CHARACTER, Character.valueOf('c'))); } - @Test - public void testString() { + @Test public void testString() { serializeAndEqualityCheck(TypedValue.ofLocal(Rep.STRING, "qwertyasdf")); } - @Test - public void testByteString() { + @Test public void testByteString() { serializeAndEqualityCheck( TypedValue.ofLocal(Rep.BYTE_STRING, - new ByteString("qwertyasdf".getBytes(StandardCharsets.UTF_8)))); + new ByteString("qwertyasdf".getBytes(UTF_8)))); + } + + @Test public void testBase64() { + byte[] bytes = "qwertyasdf".getBytes(UTF_8); + // Plain bytes get put into protobuf for simplicitly + Common.TypedValue proto = Common.TypedValue.newBuilder().setBytesValues( + com.google.protobuf.ByteString.copyFrom(bytes)) + .setType(Common.Rep.BYTE_STRING).build(); + + // But we should get back a b64-string to make sure TypedValue doesn't get confused. + Object deserializedObj = TypedValue.getSerialFromProto(proto); + assertThat(deserializedObj, is(instanceOf(String.class))); + assertEquals(new ByteString(bytes).toBase64String(), (String) deserializedObj); + + // But we should get a non-b64 byte array as the JDBC representation + deserializedObj = TypedValue.protoToJdbc(proto, GregorianCalendar.getInstance()); + assertThat(deserializedObj, is(instanceOf(byte[].class))); + assertArrayEquals(bytes, (byte[]) deserializedObj); } - @Test - public void testSqlDate() { + @Test public void testSqlDate() { // days since epoch serializeAndEqualityCheck(TypedValue.ofLocal(Rep.JAVA_SQL_DATE, 25)); } - @Test - public void testUtilDate() { + @Test public void testUtilDate() { serializeAndEqualityCheck( TypedValue.ofLocal(Rep.JAVA_UTIL_DATE, System.currentTimeMillis())); } - @Test - public void testSqlTime() { + @Test public void testSqlTime() { // millis since epoch serializeAndEqualityCheck( TypedValue.ofLocal(Rep.JAVA_SQL_TIME, 42 * 1024 * 1024)); } - @Test - public void testSqlTimestamp() { + @Test public void testSqlTimestamp() { serializeAndEqualityCheck( TypedValue.ofLocal(Rep.JAVA_SQL_TIMESTAMP, 42L * 1024 * 1024 * 1024)); } + + @Test public void testLegacyDecimalParsing() { + final BigDecimal decimal = new BigDecimal("123451234512345"); + final Calendar calendar = GregorianCalendar.getInstance(); + + // CALCITE-1103 Decimals were (incorrectly) getting serialized as normal "numbers" which + // caused them to use the numberValue field. TypedValue should still be able to handle + // values like this (but large values will be truncated and return bad values). + Common.TypedValue oldProtoStyle = Common.TypedValue.newBuilder().setType(Common.Rep.NUMBER) + .setNumberValue(decimal.longValue()).build(); + + TypedValue fromProtoTv = TypedValue.fromProto(oldProtoStyle); + Object o = fromProtoTv.toJdbc(calendar); + assertEquals(decimal, o); + } } // End TypedValueTest.java http://git-wip-us.apache.org/repos/asf/calcite/blob/aa9db8a3/avatica/server/src/test/java/org/apache/calcite/avatica/RemoteDriverTest.java ---------------------------------------------------------------------- diff --git a/avatica/server/src/test/java/org/apache/calcite/avatica/RemoteDriverTest.java b/avatica/server/src/test/java/org/apache/calcite/avatica/RemoteDriverTest.java index 50223b2..193d098 100644 --- a/avatica/server/src/test/java/org/apache/calcite/avatica/RemoteDriverTest.java +++ b/avatica/server/src/test/java/org/apache/calcite/avatica/RemoteDriverTest.java @@ -41,6 +41,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.lang.reflect.Field; +import java.math.BigDecimal; import java.sql.Connection; import java.sql.DatabaseMetaData; import java.sql.Date; @@ -1476,6 +1477,30 @@ public class RemoteDriverTest { } } + @Test public void testBigDecimalPrecision() throws Exception { + final String tableName = "decimalPrecision"; + // DECIMAL(25,5), 20 before, 5 after + BigDecimal decimal = new BigDecimal("12345123451234512345.09876"); + try (Connection conn = getLocalConnection(); + Statement stmt = conn.createStatement()) { + assertFalse(stmt.execute("DROP TABLE IF EXISTS " + tableName)); + assertFalse(stmt.execute("CREATE TABLE " + tableName + " (col1 DECIMAL(25,5))")); + + // Insert a single decimal + try (PreparedStatement pstmt = conn.prepareStatement("INSERT INTO " + tableName + + " values (?)")) { + pstmt.setBigDecimal(1, decimal); + assertEquals(1, pstmt.executeUpdate()); + } + + ResultSet results = stmt.executeQuery("SELECT * FROM " + tableName); + assertNotNull(results); + assertTrue(results.next()); + BigDecimal actualDecimal = results.getBigDecimal(1); + assertEquals(decimal, actualDecimal); + } + } + /** * Factory that creates a service based on a local JDBC connection. */
