Skip to content

Commit

Permalink
feat: support untyped NULL value parameters (#1224)
Browse files Browse the repository at this point in the history
* feat: support untyped NULL value parameters

* test: add tests for untyped null parameter values

* fix: update metadata test with the new table

* test: add expected table for PG

* refactor: move logic to ParameterStore
  • Loading branch information
olavloite committed Jun 8, 2023
1 parent 8f75d26 commit 80d2b9d
Show file tree
Hide file tree
Showing 10 changed files with 204 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ public void addBatch(String sql) throws SQLException {
@Override
public void setNull(int parameterIndex, int sqlType) throws SQLException {
checkClosed();
parameters.setParameter(parameterIndex, null, sqlType, null);
parameters.setParameter(
parameterIndex, /* value = */ null, sqlType, /* scaleOrLength = */ null);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.google.cloud.spanner.Value;
import com.google.cloud.spanner.ValueBinder;
import com.google.common.io.CharStreams;
import com.google.protobuf.NullValue;
import com.google.rpc.Code;
import java.io.IOException;
import java.io.InputStream;
Expand Down Expand Up @@ -231,6 +232,10 @@ void setParameter(
}

private void checkTypeAndValueSupported(Object value, int sqlType) throws SQLException {
if (value == null) {
// null is always supported, as we will just fall back to an untyped NULL value.
return;
}
if (!isTypeSupported(sqlType)) {
throw JdbcSqlExceptionFactory.of(
"Type " + sqlType + " is not supported", Code.INVALID_ARGUMENT);
Expand Down Expand Up @@ -775,8 +780,13 @@ private Builder setArrayValue(ValueBinder<Builder> binder, int type, Object valu
case Types.LONGVARBINARY:
case Types.BLOB:
return binder.toBytesArray(null);
default:
return binder.to(
Value.untyped(
com.google.protobuf.Value.newBuilder()
.setNullValue(NullValue.NULL_VALUE)
.build()));
}
throw JdbcSqlExceptionFactory.unsupported("Unknown/unsupported array base type: " + type);
}

if (boolean[].class.isAssignableFrom(value.getClass())) {
Expand Down Expand Up @@ -864,7 +874,9 @@ private List<Double> toDoubleList(Number[] input) {
*/
private Builder setNullValue(ValueBinder<Builder> binder, Integer sqlType) {
if (sqlType == null) {
return binder.to((String) null);
return binder.to(
Value.untyped(
com.google.protobuf.Value.newBuilder().setNullValue(NullValue.NULL_VALUE).build()));
}
switch (sqlType) {
case Types.BIGINT:
Expand Down Expand Up @@ -924,8 +936,14 @@ private Builder setNullValue(ValueBinder<Builder> binder, Integer sqlType) {
return binder.to((ByteArray) null);
case Types.VARCHAR:
return binder.to((String) null);
case JsonType.VENDOR_TYPE_NUMBER:
return binder.to(Value.json(null));
case PgJsonbType.VENDOR_TYPE_NUMBER:
return binder.to(Value.pgJsonb(null));
default:
throw new IllegalArgumentException("Unsupported sql type for setting to null: " + sqlType);
return binder.to(
Value.untyped(
com.google.protobuf.Value.newBuilder().setNullValue(NullValue.NULL_VALUE).build()));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.anyString;
import static org.mockito.Mockito.mock;
Expand All @@ -39,15 +38,13 @@
import com.google.cloud.spanner.Value;
import com.google.cloud.spanner.connection.AbstractStatementParser;
import com.google.cloud.spanner.connection.Connection;
import com.google.rpc.Code;
import java.io.ByteArrayInputStream;
import java.io.StringReader;
import java.math.BigDecimal;
import java.net.MalformedURLException;
import java.net.URL;
import java.sql.Date;
import java.sql.JDBCType;
import java.sql.PreparedStatement;
import java.sql.ResultSetMetaData;
import java.sql.SQLException;
import java.sql.Time;
Expand Down Expand Up @@ -197,7 +194,10 @@ public void testParameters() throws SQLException, MalformedURLException {
ps.setObject(35, "TEST");
ps.setObject(36, "TEST", Types.NVARCHAR);
ps.setObject(37, "TEST", Types.NVARCHAR, 20);
ps.setRef(38, null);
ps.setRowId(39, null);
ps.setShort(40, (short) 1);
ps.setSQLXML(41, null);
ps.setString(42, "TEST");
ps.setTime(43, new Time(1000L));
ps.setTime(44, new Time(1000L), Calendar.getInstance(TimeZone.getTimeZone("GMT")));
Expand All @@ -211,8 +211,6 @@ public void testParameters() throws SQLException, MalformedURLException {
ps.setObject(52, "{}", JsonType.VENDOR_TYPE_NUMBER);
ps.setObject(53, "{}", PgJsonbType.VENDOR_TYPE_NUMBER);

testSetUnsupportedTypes(ps);

JdbcParameterMetaData pmd = ps.getParameterMetaData();
assertEquals(numberOfParams, pmd.getParameterCount());
assertEquals(JdbcArray.class.getName(), pmd.getParameterClassName(1));
Expand Down Expand Up @@ -274,33 +272,9 @@ public void testParameters() throws SQLException, MalformedURLException {
}
}

private void testSetUnsupportedTypes(PreparedStatement ps) {
try {
ps.setRef(38, null);
fail("missing expected exception");
} catch (SQLException e) {
assertTrue(e instanceof JdbcSqlException);
assertEquals(Code.INVALID_ARGUMENT, ((JdbcSqlException) e).getCode());
}
try {
ps.setRowId(39, null);
fail("missing expected exception");
} catch (SQLException e) {
assertTrue(e instanceof JdbcSqlException);
assertEquals(Code.INVALID_ARGUMENT, ((JdbcSqlException) e).getCode());
}
try {
ps.setSQLXML(41, null);
fail("missing expected exception");
} catch (SQLException e) {
assertTrue(e instanceof JdbcSqlException);
assertEquals(Code.INVALID_ARGUMENT, ((JdbcSqlException) e).getCode());
}
}

@Test
public void testSetNullValues() throws SQLException {
final int numberOfParameters = 27;
final int numberOfParameters = 31;
String sql = generateSqlWithParameters(numberOfParameters);
try (JdbcPreparedStatement ps = new JdbcPreparedStatement(createMockConnection(), sql)) {
int index = 0;
Expand Down Expand Up @@ -331,6 +305,10 @@ public void testSetNullValues() throws SQLException {
ps.setNull(++index, Types.BIT);
ps.setNull(++index, Types.VARBINARY);
ps.setNull(++index, Types.VARCHAR);
ps.setNull(++index, JsonType.VENDOR_TYPE_NUMBER);
ps.setNull(++index, PgJsonbType.VENDOR_TYPE_NUMBER);
ps.setNull(++index, Types.OTHER);
ps.setNull(++index, Types.NULL);
assertEquals(numberOfParameters, index);

JdbcParameterMetaData pmd = ps.getParameterMetaData();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import com.google.cloud.spanner.MockSpannerServiceImpl;
import com.google.cloud.spanner.MockSpannerServiceImpl.StatementResult;
import com.google.cloud.spanner.Statement;
import com.google.cloud.spanner.Value;
import com.google.cloud.spanner.connection.SpannerPool;
import com.google.cloud.spanner.jdbc.JdbcSqlExceptionFactory.JdbcSqlBatchUpdateException;
import io.grpc.Server;
Expand All @@ -36,6 +37,7 @@
import java.sql.DriverManager;
import java.sql.PreparedStatement;
import java.sql.SQLException;
import java.sql.Types;
import java.util.Arrays;
import java.util.Collection;
import org.junit.After;
Expand Down Expand Up @@ -193,4 +195,69 @@ public void testExecuteBatch_withException() throws SQLException {
}
}
}

@Test
public void testInsertUntypedNullValues() throws SQLException {
mockSpanner.putStatementResult(
StatementResult.update(
Statement.newBuilder(
"insert into all_nullable_types (ColInt64, ColFloat64, ColBool, ColString, ColBytes, ColDate, ColTimestamp, ColNumeric, ColJson, ColInt64Array, ColFloat64Array, ColBoolArray, ColStringArray, ColBytesArray, ColDateArray, ColTimestampArray, ColNumericArray, ColJsonArray) "
+ "values (@p1, @p2, @p3, @p4, @p5, @p6, @p7, @p8, @p9, @p10, @p11, @p12, @p13, @p14, @p15, @p16, @p17, @p18)")
.bind("p1")
.to((Value) null)
.bind("p2")
.to((Value) null)
.bind("p3")
.to((Value) null)
.bind("p4")
.to((Value) null)
.bind("p5")
.to((Value) null)
.bind("p6")
.to((Value) null)
.bind("p7")
.to((Value) null)
.bind("p8")
.to((Value) null)
.bind("p9")
.to((Value) null)
.bind("p10")
.to((Value) null)
.bind("p11")
.to((Value) null)
.bind("p12")
.to((Value) null)
.bind("p13")
.to((Value) null)
.bind("p14")
.to((Value) null)
.bind("p15")
.to((Value) null)
.bind("p16")
.to((Value) null)
.bind("p17")
.to((Value) null)
.bind("p18")
.to((Value) null)
.build(),
1L));
try (Connection connection = createConnection()) {
for (int type : new int[] {Types.OTHER, Types.NULL}) {
try (PreparedStatement statement =
connection.prepareStatement(
"insert into all_nullable_types ("
+ "ColInt64, ColFloat64, ColBool, ColString, ColBytes, ColDate, ColTimestamp, ColNumeric, ColJson, "
+ "ColInt64Array, ColFloat64Array, ColBoolArray, ColStringArray, ColBytesArray, ColDateArray, ColTimestampArray, ColNumericArray, ColJsonArray) "
+ "values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)")) {
for (int param = 1;
param <= statement.getParameterMetaData().getParameterCount();
param++) {
statement.setNull(param, type);
}
assertEquals(1, statement.executeUpdate());
}
mockSpanner.clearRequests();
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,8 @@ private IndexInfo(
new IndexInfo("TableWithRef", false, "PRIMARY_KEY", 1, "Id", "A"),
new IndexInfo("TableWithRef", true, "FOREIGN_KEY", 1, "RefFloat", "A"),
new IndexInfo("TableWithRef", true, "FOREIGN_KEY", 2, "RefString", "A"),
new IndexInfo("TableWithRef", true, "FOREIGN_KEY", 3, "RefDate", "A"));
new IndexInfo("TableWithRef", true, "FOREIGN_KEY", 3, "RefDate", "A"),
new IndexInfo("all_nullable_types", false, "PRIMARY_KEY", 1, "ColInt64", "A"));

@Test
public void testGetIndexInfo() throws SQLException {
Expand Down Expand Up @@ -860,7 +861,8 @@ private Table(String name, String type) {
new Table("SingersView", "VIEW"),
new Table("Songs"),
new Table("TableWithAllColumnTypes"),
new Table("TableWithRef"));
new Table("TableWithRef"),
new Table("all_nullable_types"));

@Test
public void testGetTables() throws SQLException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,7 @@ private IndexInfo(
new IndexInfo("albums", false, "PRIMARY_KEY", 1, "singerid", "A"),
new IndexInfo("albums", false, "PRIMARY_KEY", 2, "albumid", "A"),
new IndexInfo("albums", true, "albumsbyalbumtitle", 1, "albumtitle", "A"),
new IndexInfo("all_nullable_types", false, "PRIMARY_KEY", 1, "colint64", "A"),
new IndexInfo("concerts", false, "PRIMARY_KEY", 1, "venueid", "A"),
new IndexInfo("concerts", false, "PRIMARY_KEY", 2, "singerid", "A"),
new IndexInfo("concerts", false, "PRIMARY_KEY", 3, "concertdate", "A"),
Expand Down Expand Up @@ -790,6 +791,7 @@ private Table(String name, String type) {
private static final List<Table> EXPECTED_TABLES =
Arrays.asList(
new Table("albums"),
new Table("all_nullable_types"),
new Table("concerts"),
new Table("singers"),
// TODO: Enable when views are supported for PostgreSQL dialect databases.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1294,6 +1294,39 @@ public void test12_InsertReturningTestData() throws SQLException {
}
}

@Test
public void test13_InsertUntypedNullValues() throws SQLException {
try (Connection connection = createConnection(env, database)) {
try (PreparedStatement preparedStatement =
connection.prepareStatement(
"insert into all_nullable_types ("
+ "ColInt64, ColFloat64, ColBool, ColString, ColBytes, ColDate, ColTimestamp, ColNumeric, ColJson, "
+ "ColInt64Array, ColFloat64Array, ColBoolArray, ColStringArray, ColBytesArray, ColDateArray, ColTimestampArray, ColNumericArray, ColJsonArray) "
+ "values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)")) {
for (int param = 1;
param <= preparedStatement.getParameterMetaData().getParameterCount();
param++) {
preparedStatement.setNull(param, Types.OTHER);
}
if (getDialect() == Dialect.POSTGRESQL) {
// PostgreSQL-dialect databases do not allow NULLs in primary keys.
preparedStatement.setLong(1, 1L);
}
assertEquals(1, preparedStatement.executeUpdate());

// Verify that calling preparedStatement.setObject(index, null) works.
for (int param = 1;
param <= preparedStatement.getParameterMetaData().getParameterCount();
param++) {
preparedStatement.setObject(param, null);
}
// We need a different primary key value to insert another row.
preparedStatement.setLong(1, 2L);
assertEquals(1, preparedStatement.executeUpdate());
}
}
}

private List<String> readValuesFromFile(String filename) {
StringBuilder builder = new StringBuilder();
try (InputStream stream = ITJdbcPreparedStatementTest.class.getResourceAsStream(filename)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,29 @@ CREATE TABLE TableWithAllColumnTypes (
) PRIMARY KEY (ColInt64)
;

CREATE TABLE all_nullable_types (
ColInt64 INT64,
ColFloat64 FLOAT64,
ColBool BOOL,
ColString STRING(100),
ColBytes BYTES(100),
ColDate DATE,
ColTimestamp TIMESTAMP,
ColNumeric NUMERIC,
ColJson JSON,

ColInt64Array ARRAY<INT64>,
ColFloat64Array ARRAY<FLOAT64>,
ColBoolArray ARRAY<BOOL>,
ColStringArray ARRAY<STRING(100)>,
ColBytesArray ARRAY<BYTES(100)>,
ColDateArray ARRAY<DATE>,
ColTimestampArray ARRAY<TIMESTAMP>,
ColNumericArray ARRAY<NUMERIC>,
ColJsonArray ARRAY<JSON>,
) PRIMARY KEY (ColInt64)
;

CREATE TABLE TableWithRef (
Id INT64 NOT NULL,
RefFloat FLOAT64 NOT NULL,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,29 @@ CREATE TABLE TableWithAllColumnTypes (
) PRIMARY KEY (ColInt64)
;

CREATE TABLE all_nullable_types (
ColInt64 INT64,
ColFloat64 FLOAT64,
ColBool BOOL,
ColString STRING(100),
ColBytes BYTES(100),
ColDate DATE,
ColTimestamp TIMESTAMP,
ColNumeric NUMERIC,
ColJson JSON,

ColInt64Array ARRAY<INT64>,
ColFloat64Array ARRAY<FLOAT64>,
ColBoolArray ARRAY<BOOL>,
ColStringArray ARRAY<STRING(100)>,
ColBytesArray ARRAY<BYTES(100)>,
ColDateArray ARRAY<DATE>,
ColTimestampArray ARRAY<TIMESTAMP>,
ColNumericArray ARRAY<NUMERIC>,
ColJsonArray ARRAY<JSON>,
) PRIMARY KEY (ColInt64)
;

CREATE TABLE TableWithRef (
Id INT64 NOT NULL,
RefFloat FLOAT64 NOT NULL,
Expand Down
Loading

0 comments on commit 80d2b9d

Please sign in to comment.