Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: support getShort for DATA_TYPE in TypeInfo #1691

Merged
merged 3 commits into from
Aug 6, 2024
Merged

fix: support getShort for DATA_TYPE in TypeInfo #1691

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d0cade2
fix: support getShort for DATA_TYPE in TypeInfo
olavloite Jul 25, 2024
3ad35f0
chore: cleanup
olavloite Jul 25, 2024
a9da755
Merge branch 'main' into support-short-vendor-types
olavloite Aug 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/main/java/com/google/cloud/spanner/jdbc/JdbcDatabaseMetaData.java
View file
Open in desktop
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
Open in desktop
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
28 changes: 26 additions & 2 deletions src/main/java/com/google/cloud/spanner/jdbc/JdbcResultSet.java
View file
Open in desktop
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,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));
Expand Down
6 changes: 6 additions & 0 deletions src/main/java/com/google/cloud/spanner/jdbc/JsonType.java
View file
Open in desktop
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
6 changes: 6 additions & 0 deletions src/main/java/com/google/cloud/spanner/jdbc/PgJsonbType.java
View file
Open in desktop
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.SQLType;

public class PgJsonbType implements SQLType {
Expand All @@ -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() {}

Expand Down
6 changes: 6 additions & 0 deletions src/main/java/com/google/cloud/spanner/jdbc/ProtoEnumType.java
View file
Open in desktop
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
Open in desktop
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
29 changes: 28 additions & 1 deletion src/test/java/com/google/cloud/spanner/jdbc/JdbcDatabaseMetaDataTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
Loading
Loading