Skip to content

Commit

Permalink
fix: support getShort for DATA_TYPE in TypeInfo
Browse files Browse the repository at this point in the history 
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 #1688
  • Loading branch information
@olavloite
olavloite committed Jul 25, 2024
1 parent 6a57b49 commit d0cade2
Show file tree
Hide file tree
Showing 9 changed files with 161 additions and 17 deletions.
5 changes: 4 additions & 1 deletion src/main/java/com/google/cloud/spanner/jdbc/JdbcDatabaseMetaData.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
38 changes: 33 additions & 5 deletions src/main/java/com/google/cloud/spanner/jdbc/JdbcParameterStore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand All @@ -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));
Expand Down Expand Up @@ -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[]) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
Expand All @@ -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()
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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(
Expand Down
27 changes: 25 additions & 2 deletions src/main/java/com/google/cloud/spanner/jdbc/JdbcResultSet.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -327,6 +345,11 @@ 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().
return isNull ? 0 : (short) spanner.getLong(spannerIndex);
}
return isNull ? 0 : checkedCastToShort(spanner.getLong(spannerIndex));
case NUMERIC:
return isNull ? 0 : checkedCastToShort(spanner.getBigDecimal(spannerIndex));
Expand Down
6 changes: 6 additions & 0 deletions src/main/java/com/google/cloud/spanner/jdbc/JsonType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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() {}

Expand Down
8 changes: 7 additions & 1 deletion src/main/java/com/google/cloud/spanner/jdbc/PgJsonbType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,22 @@
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 {
public static final PgJsonbType INSTANCE = new PgJsonbType();
/**
* Spanner/Spangres does not have any type numbers, but the code values are unique. Add 200,000 to
* avoid conflicts with the type numbers in java.sql.Types. Native Cloud Spanner types already use
* the range starting at 100,000 (see {@link JsonType}).
* the range starting at 16,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() {}

Expand Down
6 changes: 6 additions & 0 deletions src/main/java/com/google/cloud/spanner/jdbc/ProtoEnumType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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() {}

Expand Down
6 changes: 6 additions & 0 deletions src/main/java/com/google/cloud/spanner/jdbc/ProtoMessageType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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() {}

Expand Down
31 changes: 29 additions & 2 deletions src/test/java/com/google/cloud/spanner/jdbc/JdbcDatabaseMetaDataTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -482,25 +482,52 @@ 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"));
assertThat(rs.next(), is(false));
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();
assertThat(rsmd.getColumnCount(), is(equalTo(18)));
}
Expand Down
Loading

0 comments on commit d0cade2

Please sign in to comment.