fix: support getShort for DATA_TYPE in TypeInfo (googleapis#1691)

* fix: support getShort for DATA_TYPE in TypeInfo

The ResultSet that is returned by DatabaseMetadata#getTypeInfo has
a column at index 2 with the name DATA_TYPE. This field should
contain one of the java.sql.Types constants, or a vendor-specific
type code. The JDBC specification states that this column should
be a `short` (although the constants in java.sql.Types are of
type `int`).
Cloud Spanner (at the time of writing) does not support any int16
fields. The type code is therefore returned as an int64. The codes
that are used for vendor-specific types by Spanner exceed the max
value of a `short`, and therefore resulted in an OUT_OF_RANGE
exception if you tried to call `ResultSet#getShort(int)` on
this column for any of the Spanner-specific types (e.g. JSON).

This change fixes that by adding an additional vendor type code
for these types that does fit in a `short`. This value is
returned when `getShort(int)` is called on the ResultSet.

Fixes googleapis#1688

* chore: cleanup

Author: olavloite

src/main/java/com/google/cloud/spanner/jdbc/JdbcDatabaseMetaData.java

@@ -26,6 +26,7 @@
 import com.google.cloud.spanner.Type.StructField;
 import com.google.cloud.spanner.connection.Connection.InternalMetadataQuery;
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableSet;
 import java.io.BufferedReader;
 import java.io.InputStream;
 import java.io.InputStreamReader;
@@ -1284,7 +1285,9 @@ public ResultSet getTypeInfo() {
                     .set("NUM_PREC_RADIX")
                     .to(10)
                     .build(),
-                getJsonType(connection.getDialect()))));
+                getJsonType(connection.getDialect()))),
+        // Allow column 2 to be cast to short without any range checks.
+        ImmutableSet.of(2));
   }
 
   private Struct getJsonType(Dialect dialect) {

src/main/java/com/google/cloud/spanner/jdbc/JdbcParameterStore.java

@@ -284,9 +284,13 @@ private boolean isTypeSupported(int sqlType) {
       case Types.NUMERIC:
       case Types.DECIMAL:
       case JsonType.VENDOR_TYPE_NUMBER:
+      case JsonType.SHORT_VENDOR_TYPE_NUMBER:
       case PgJsonbType.VENDOR_TYPE_NUMBER:
+      case PgJsonbType.SHORT_VENDOR_TYPE_NUMBER:
       case ProtoMessageType.VENDOR_TYPE_NUMBER:
+      case ProtoMessageType.SHORT_VENDOR_TYPE_NUMBER:
       case ProtoEnumType.VENDOR_TYPE_NUMBER:
+      case ProtoEnumType.SHORT_VENDOR_TYPE_NUMBER:
         return true;
     }
     return false;
@@ -348,19 +352,23 @@ private boolean isValidTypeAndValue(Object value, int sqlType) {
       case Types.NCLOB:
         return value instanceof NClob || value instanceof Reader;
       case JsonType.VENDOR_TYPE_NUMBER:
+      case JsonType.SHORT_VENDOR_TYPE_NUMBER:
         return value instanceof String
             || value instanceof InputStream
             || value instanceof Reader
             || (value instanceof Value && ((Value) value).getType().getCode() == Type.Code.JSON);
       case PgJsonbType.VENDOR_TYPE_NUMBER:
+      case PgJsonbType.SHORT_VENDOR_TYPE_NUMBER:
         return value instanceof String
             || value instanceof InputStream
             || value instanceof Reader
             || (value instanceof Value
                 && ((Value) value).getType().getCode() == Type.Code.PG_JSONB);
       case ProtoMessageType.VENDOR_TYPE_NUMBER:
+      case ProtoMessageType.SHORT_VENDOR_TYPE_NUMBER:
         return value instanceof AbstractMessage || value instanceof byte[];
       case ProtoEnumType.VENDOR_TYPE_NUMBER:
+      case ProtoEnumType.SHORT_VENDOR_TYPE_NUMBER:
         return value instanceof ProtocolMessageEnum || value instanceof Number;
     }
     return false;
@@ -449,7 +457,12 @@ private Builder setSingleValue(ValueBinder<Builder> binder, Object value, Intege
   /** Set a JDBC parameter value on a Spanner {@link Statement} with a known SQL type. */
   private Builder setParamWithKnownType(ValueBinder<Builder> binder, Object value, Integer sqlType)
       throws SQLException {
-    switch (sqlType) {
+    if (sqlType == null) {
+      return null;
+    }
+    int type = sqlType;
+
+    switch (type) {
       case Types.BIT:
       case Types.BOOLEAN:
         if (value instanceof Boolean) {
@@ -522,7 +535,9 @@ private Builder setParamWithKnownType(ValueBinder<Builder> binder, Object value,
         }
         return binder.to(stringValue);
       case JsonType.VENDOR_TYPE_NUMBER:
+      case JsonType.SHORT_VENDOR_TYPE_NUMBER:
       case PgJsonbType.VENDOR_TYPE_NUMBER:
+      case PgJsonbType.SHORT_VENDOR_TYPE_NUMBER:
         String jsonValue;
         if (value instanceof String) {
           jsonValue = (String) value;
@@ -534,7 +549,8 @@ private Builder setParamWithKnownType(ValueBinder<Builder> binder, Object value,
           throw JdbcSqlExceptionFactory.of(
               value + " is not a valid JSON value", Code.INVALID_ARGUMENT);
         }
-        if (sqlType == PgJsonbType.VENDOR_TYPE_NUMBER) {
+        if (type == PgJsonbType.VENDOR_TYPE_NUMBER
+            || type == PgJsonbType.SHORT_VENDOR_TYPE_NUMBER) {
           return binder.to(Value.pgJsonb(jsonValue));
         }
         return binder.to(Value.json(jsonValue));
@@ -631,6 +647,7 @@ private Builder setParamWithKnownType(ValueBinder<Builder> binder, Object value,
         }
         throw JdbcSqlExceptionFactory.of(value + " is not a valid clob", Code.INVALID_ARGUMENT);
       case ProtoMessageType.VENDOR_TYPE_NUMBER:
+      case ProtoMessageType.SHORT_VENDOR_TYPE_NUMBER:
         if (value instanceof AbstractMessage) {
           return binder.to((AbstractMessage) value);
         } else if (value instanceof byte[]) {
@@ -640,6 +657,7 @@ private Builder setParamWithKnownType(ValueBinder<Builder> binder, Object value,
               value + " is not a valid PROTO value", Code.INVALID_ARGUMENT);
         }
       case ProtoEnumType.VENDOR_TYPE_NUMBER:
+      case ProtoEnumType.SHORT_VENDOR_TYPE_NUMBER:
         if (value instanceof ProtocolMessageEnum) {
           return binder.to((ProtocolMessageEnum) value);
         } else if (value instanceof Number) {
@@ -809,8 +827,10 @@ private Builder setArrayValue(ValueBinder<Builder> binder, int type, Object valu
         case Types.NCLOB:
           return binder.toStringArray(null);
         case JsonType.VENDOR_TYPE_NUMBER:
+        case JsonType.SHORT_VENDOR_TYPE_NUMBER:
           return binder.toJsonArray(null);
         case PgJsonbType.VENDOR_TYPE_NUMBER:
+        case PgJsonbType.SHORT_VENDOR_TYPE_NUMBER:
           return binder.toPgJsonbArray(null);
         case Types.DATE:
           return binder.toDateArray(null);
@@ -825,7 +845,9 @@ private Builder setArrayValue(ValueBinder<Builder> binder, int type, Object valu
         case Types.BLOB:
           return binder.toBytesArray(null);
         case ProtoMessageType.VENDOR_TYPE_NUMBER:
+        case ProtoMessageType.SHORT_VENDOR_TYPE_NUMBER:
         case ProtoEnumType.VENDOR_TYPE_NUMBER:
+        case ProtoEnumType.SHORT_VENDOR_TYPE_NUMBER:
           return binder.to(
               Value.untyped(
                   com.google.protobuf.Value.newBuilder()
@@ -886,9 +908,10 @@ private Builder setArrayValue(ValueBinder<Builder> binder, int type, Object valu
     } else if (Timestamp[].class.isAssignableFrom(value.getClass())) {
       return binder.toTimestampArray(JdbcTypeConverter.toGoogleTimestamps((Timestamp[]) value));
     } else if (String[].class.isAssignableFrom(value.getClass())) {
-      if (type == JsonType.VENDOR_TYPE_NUMBER) {
+      if (type == JsonType.VENDOR_TYPE_NUMBER || type == JsonType.SHORT_VENDOR_TYPE_NUMBER) {
         return binder.toJsonArray(Arrays.asList((String[]) value));
-      } else if (type == PgJsonbType.VENDOR_TYPE_NUMBER) {
+      } else if (type == PgJsonbType.VENDOR_TYPE_NUMBER
+          || type == PgJsonbType.SHORT_VENDOR_TYPE_NUMBER) {
         return binder.toPgJsonbArray(Arrays.asList((String[]) value));
       } else {
         return binder.toStringArray(Arrays.asList((String[]) value));
@@ -992,11 +1015,13 @@ private Builder setNullValue(ValueBinder<Builder> binder, Integer sqlType) {
           Value.untyped(
               com.google.protobuf.Value.newBuilder().setNullValue(NullValue.NULL_VALUE).build()));
     }
-    switch (sqlType) {
+    int type = sqlType;
+    switch (type) {
       case Types.BIGINT:
         return binder.to((Long) null);
       case Types.BINARY:
       case ProtoMessageType.VENDOR_TYPE_NUMBER:
+      case ProtoEnumType.SHORT_VENDOR_TYPE_NUMBER:
         return binder.to((ByteArray) null);
       case Types.BLOB:
         return binder.to((ByteArray) null);
@@ -1021,6 +1046,7 @@ private Builder setNullValue(ValueBinder<Builder> binder, Integer sqlType) {
         return binder.to((Double) null);
       case Types.INTEGER:
       case ProtoEnumType.VENDOR_TYPE_NUMBER:
+      case ProtoMessageType.SHORT_VENDOR_TYPE_NUMBER:
         return binder.to((Long) null);
       case Types.LONGNVARCHAR:
         return binder.to((String) null);
@@ -1052,8 +1078,10 @@ private Builder setNullValue(ValueBinder<Builder> binder, Integer sqlType) {
       case Types.VARCHAR:
         return binder.to((String) null);
       case JsonType.VENDOR_TYPE_NUMBER:
+      case JsonType.SHORT_VENDOR_TYPE_NUMBER:
         return binder.to(Value.json(null));
       case PgJsonbType.VENDOR_TYPE_NUMBER:
+      case PgJsonbType.SHORT_VENDOR_TYPE_NUMBER:
         return binder.to(Value.pgJsonb(null));
       default:
         return binder.to(

src/main/java/com/google/cloud/spanner/jdbc/JdbcResultSet.java

@@ -26,6 +26,7 @@
 import com.google.cloud.spanner.connection.PartitionedQueryResultSet;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import java.io.ByteArrayInputStream;
 import java.io.InputStream;
 import java.io.Reader;
@@ -57,8 +58,16 @@
 class JdbcResultSet extends AbstractJdbcResultSet {
 
   static JdbcResultSet of(com.google.cloud.spanner.ResultSet resultSet) {
-    Preconditions.checkNotNull(resultSet);
-    return new JdbcResultSet(null, resultSet);
+    return of(resultSet, ImmutableSet.of());
+  }
+
+  static JdbcResultSet of(
+      com.google.cloud.spanner.ResultSet resultSet,
+      ImmutableSet<Integer> columnsAllowedUncheckedLongCastToShort) {
+    return new JdbcResultSet(
+        null,
+        Preconditions.checkNotNull(resultSet),
+        Preconditions.checkNotNull(columnsAllowedUncheckedLongCastToShort));
   }
 
   static JdbcResultSet of(Statement statement, com.google.cloud.spanner.ResultSet resultSet) {
@@ -129,10 +138,19 @@ public Struct next() {
   private boolean nextCalledForMetaData = false;
   private boolean nextCalledForMetaDataResult = false;
   private long currentRow = 0L;
+  private final ImmutableSet<Integer> columnsAllowedUncheckedLongCastToShort;
 
   JdbcResultSet(Statement statement, com.google.cloud.spanner.ResultSet spanner) {
+    this(statement, spanner, ImmutableSet.of());
+  }
+
+  JdbcResultSet(
+      Statement statement,
+      com.google.cloud.spanner.ResultSet spanner,
+      ImmutableSet<Integer> columnsAllowedUncheckedLongCastToShort) {
     super(spanner);
     this.statement = statement;
+    this.columnsAllowedUncheckedLongCastToShort = columnsAllowedUncheckedLongCastToShort;
   }
 
   void checkClosedAndValidRow() throws SQLException {
@@ -327,6 +345,12 @@ public short getShort(int columnIndex) throws SQLException {
             : checkedCastToShort(Double.valueOf(spanner.getDouble(spannerIndex)).longValue());
       case INT64:
       case ENUM:
+        if (this.columnsAllowedUncheckedLongCastToShort.contains(columnIndex)) {
+          // This is used to allow frameworks that call getShort(int) on the ResultSet that is
+          // returned by DatabaseMetadata#getTypeInfo() to get the type code as a short, even when
+          // the value is out of range for a short.
+          return isNull ? 0 : (short) spanner.getLong(spannerIndex);
+        }
         return isNull ? 0 : checkedCastToShort(spanner.getLong(spannerIndex));
       case NUMERIC:
         return isNull ? 0 : checkedCastToShort(spanner.getBigDecimal(spannerIndex));

src/main/java/com/google/cloud/spanner/jdbc/JsonType.java

@@ -17,6 +17,7 @@
 package com.google.cloud.spanner.jdbc;
 
 import com.google.spanner.v1.TypeCode;
+import java.sql.DatabaseMetaData;
 import java.sql.PreparedStatement;
 import java.sql.SQLType;
 
@@ -31,6 +32,11 @@ public class JsonType implements SQLType {
    * conflicts with the type numbers in java.sql.Types.
    */
   public static final int VENDOR_TYPE_NUMBER = 100_000 + TypeCode.JSON_VALUE;
+  /**
+   * Define a short type number as well, as this is what is expected to be returned in {@link
+   * DatabaseMetaData#getTypeInfo()}.
+   */
+  public static final short SHORT_VENDOR_TYPE_NUMBER = (short) VENDOR_TYPE_NUMBER;
 
   private JsonType() {}
 

src/main/java/com/google/cloud/spanner/jdbc/PgJsonbType.java

@@ -17,6 +17,7 @@
 package com.google.cloud.spanner.jdbc;
 
 import com.google.spanner.v1.TypeCode;
+import java.sql.DatabaseMetaData;
 import java.sql.SQLType;
 
 public class PgJsonbType implements SQLType {
@@ -27,6 +28,11 @@ public class PgJsonbType implements SQLType {
    * the range starting at 100,000 (see {@link JsonType}).
    */
   public static final int VENDOR_TYPE_NUMBER = 200_000 + TypeCode.JSON_VALUE;
+  /**
+   * Define a short type number as well, as this is what is expected to be returned in {@link
+   * DatabaseMetaData#getTypeInfo()}.
+   */
+  public static final short SHORT_VENDOR_TYPE_NUMBER = (short) VENDOR_TYPE_NUMBER;
 
   private PgJsonbType() {}
 

src/main/java/com/google/cloud/spanner/jdbc/ProtoEnumType.java

@@ -17,6 +17,7 @@
 package com.google.cloud.spanner.jdbc;
 
 import com.google.spanner.v1.TypeCode;
+import java.sql.DatabaseMetaData;
 import java.sql.PreparedStatement;
 import java.sql.SQLType;
 
@@ -31,6 +32,11 @@ public class ProtoEnumType implements SQLType {
    * conflicts with the type numbers in java.sql.Types.
    */
   public static final int VENDOR_TYPE_NUMBER = 100_000 + TypeCode.ENUM_VALUE;
+  /**
+   * Define a short type number as well, as this is what is expected to be returned in {@link
+   * DatabaseMetaData#getTypeInfo()}.
+   */
+  public static final short SHORT_VENDOR_TYPE_NUMBER = (short) VENDOR_TYPE_NUMBER;
 
   private ProtoEnumType() {}
 

src/main/java/com/google/cloud/spanner/jdbc/ProtoMessageType.java

@@ -17,6 +17,7 @@
 package com.google.cloud.spanner.jdbc;
 
 import com.google.spanner.v1.TypeCode;
+import java.sql.DatabaseMetaData;
 import java.sql.PreparedStatement;
 import java.sql.SQLType;
 
@@ -31,6 +32,11 @@ public class ProtoMessageType implements SQLType {
    * conflicts with the type numbers in java.sql.Types.
    */
   public static final int VENDOR_TYPE_NUMBER = 100_000 + TypeCode.PROTO_VALUE;
+  /**
+   * Define a short type number as well, as this is what is expected to be returned in {@link
+   * DatabaseMetaData#getTypeInfo()}.
+   */
+  public static final short SHORT_VENDOR_TYPE_NUMBER = (short) VENDOR_TYPE_NUMBER;
 
   private ProtoMessageType() {}
 

src/test/java/com/google/cloud/spanner/jdbc/JdbcDatabaseMetaDataTest.java

@@ -475,24 +475,51 @@ public void testGetTypeInfo() throws SQLException {
       try (ResultSet rs = meta.getTypeInfo()) {
         assertTrue(rs.next());
         assertEquals("STRING", rs.getString("TYPE_NAME"));
+        assertEquals(Types.NVARCHAR, rs.getInt("DATA_TYPE"));
+        assertEquals(Types.NVARCHAR, rs.getShort("DATA_TYPE"));
         assertTrue(rs.next());
         assertEquals("INT64", rs.getString("TYPE_NAME"));
+        assertEquals(Types.BIGINT, rs.getInt("DATA_TYPE"));
+        assertEquals(Types.BIGINT, rs.getShort("DATA_TYPE"));
         assertTrue(rs.next());
         assertEquals("BYTES", rs.getString("TYPE_NAME"));
+        assertEquals(Types.BINARY, rs.getInt("DATA_TYPE"));
+        assertEquals(Types.BINARY, rs.getShort("DATA_TYPE"));
         assertTrue(rs.next());
         assertEquals("FLOAT32", rs.getString("TYPE_NAME"));
+        assertEquals(Types.REAL, rs.getInt("DATA_TYPE"));
+        assertEquals(Types.REAL, rs.getShort("DATA_TYPE"));
         assertTrue(rs.next());
         assertEquals("FLOAT64", rs.getString("TYPE_NAME"));
+        assertEquals(Types.DOUBLE, rs.getInt("DATA_TYPE"));
+        assertEquals(Types.DOUBLE, rs.getShort("DATA_TYPE"));
         assertTrue(rs.next());
         assertEquals("BOOL", rs.getString("TYPE_NAME"));
+        assertEquals(Types.BOOLEAN, rs.getInt("DATA_TYPE"));
+        assertEquals(Types.BOOLEAN, rs.getShort("DATA_TYPE"));
         assertTrue(rs.next());
         assertEquals("DATE", rs.getString("TYPE_NAME"));
+        assertEquals(Types.DATE, rs.getInt("DATA_TYPE"));
+        assertEquals(Types.DATE, rs.getShort("DATA_TYPE"));
         assertTrue(rs.next());
         assertEquals("TIMESTAMP", rs.getString("TYPE_NAME"));
+        assertEquals(Types.TIMESTAMP, rs.getInt("DATA_TYPE"));
+        assertEquals(Types.TIMESTAMP, rs.getShort("DATA_TYPE"));
         assertTrue(rs.next());
         assertEquals("NUMERIC", rs.getString("TYPE_NAME"));
+        assertEquals(Types.NUMERIC, rs.getInt("DATA_TYPE"));
+        assertEquals(Types.NUMERIC, rs.getShort("DATA_TYPE"));
         assertTrue(rs.next());
-        assertEquals(dialect == Dialect.POSTGRESQL ? "JSONB" : "JSON", rs.getString("TYPE_NAME"));
+        if (dialect == Dialect.POSTGRESQL) {
+          assertEquals("JSONB", rs.getString("TYPE_NAME"));
+          assertEquals(PgJsonbType.VENDOR_TYPE_NUMBER, rs.getInt("DATA_TYPE"));
+          assertEquals(PgJsonbType.SHORT_VENDOR_TYPE_NUMBER, rs.getShort("DATA_TYPE"));
+        } else {
+          assertEquals("JSON", rs.getString("TYPE_NAME"));
+          assertEquals(JsonType.VENDOR_TYPE_NUMBER, rs.getInt("DATA_TYPE"));
+          assertEquals(JsonType.SHORT_VENDOR_TYPE_NUMBER, rs.getShort("DATA_TYPE"));
+        }
+
         assertFalse(rs.next());
         ResultSetMetaData rsmd = rs.getMetaData();
         assertEquals(18, rsmd.getColumnCount());