From 9f6a573e275195eb0d5a52580a4d7e3b0a945dcf Mon Sep 17 00:00:00 2001 From: Rajat Bhatta Date: Mon, 22 Aug 2022 17:09:15 +0000 Subject: [PATCH 01/10] feat: add tests for DML with Returning clause --- .../cloud/spanner/jdbc/JdbcStatement.java | 2 +- .../cloud/spanner/jdbc/JdbcStatementTest.java | 79 +++++++++- .../jdbc/it/ITJdbcPreparedStatementTest.java | 137 ++++++++++++++++++ 3 files changed, 215 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/google/cloud/spanner/jdbc/JdbcStatement.java b/src/main/java/com/google/cloud/spanner/jdbc/JdbcStatement.java index f37eb21a..42ef98c0 100644 --- a/src/main/java/com/google/cloud/spanner/jdbc/JdbcStatement.java +++ b/src/main/java/com/google/cloud/spanner/jdbc/JdbcStatement.java @@ -87,7 +87,7 @@ public long executeLargeUpdate(String sql) throws SQLException { switch (result.getResultType()) { case RESULT_SET: throw JdbcSqlExceptionFactory.of( - "The statement is not an update or DDL statement", Code.INVALID_ARGUMENT); + "The statement is not a normal update or DDL statement", Code.INVALID_ARGUMENT); case UPDATE_COUNT: return result.getUpdateCount(); case NO_RESULT: diff --git a/src/test/java/com/google/cloud/spanner/jdbc/JdbcStatementTest.java b/src/test/java/com/google/cloud/spanner/jdbc/JdbcStatementTest.java index 3ee9edcb..bf8d8500 100644 --- a/src/test/java/com/google/cloud/spanner/jdbc/JdbcStatementTest.java +++ b/src/test/java/com/google/cloud/spanner/jdbc/JdbcStatementTest.java @@ -17,6 +17,10 @@ package com.google.cloud.spanner.jdbc; import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.Mockito.anyList; import static org.mockito.Mockito.mock; @@ -53,6 +57,8 @@ public class JdbcStatementTest { private static final String SELECT = "SELECT 1"; private static final String UPDATE = "UPDATE FOO SET BAR=1 WHERE BAZ=2"; private static final String LARGE_UPDATE = "UPDATE FOO SET BAR=1 WHERE 1=1"; + private static final String DML_RETURNING_GSQL = "UPDATE FOO SET BAR=1 WHERE 1=1 THEN RETURN *"; + private static final String DML_RETURNING_PG = "UPDATE FOO SET BAR=1 WHERE 1=1 RETURNING *"; private static final String DDL = "CREATE INDEX FOO ON BAR(ID)"; @Parameter public Dialect dialect; @@ -62,11 +68,21 @@ public static Object[] data() { return Dialect.values(); } + private String getDmlReturningString() { + if (dialect == Dialect.GOOGLE_STANDARD_SQL) { + return DML_RETURNING_GSQL; + } else { + return DML_RETURNING_PG; + } + } + @SuppressWarnings("unchecked") private JdbcStatement createStatement() throws SQLException { Connection spanner = mock(Connection.class); when(spanner.getDialect()).thenReturn(dialect); + final String DML_RETURNING = getDmlReturningString(); + com.google.cloud.spanner.ResultSet resultSet = mock(com.google.cloud.spanner.ResultSet.class); when(resultSet.next()).thenReturn(true, false); when(resultSet.getColumnType(0)).thenReturn(Type.int64()); @@ -88,6 +104,18 @@ private JdbcStatement createStatement() throws SQLException { when(spanner.execute(com.google.cloud.spanner.Statement.of(LARGE_UPDATE))) .thenReturn(largeUpdateResult); + com.google.cloud.spanner.ResultSet dmlReturningResultSet = + mock(com.google.cloud.spanner.ResultSet.class); + when(dmlReturningResultSet.next()).thenReturn(true, false); + when(dmlReturningResultSet.getColumnCount()).thenReturn(1); + when(dmlReturningResultSet.getColumnType(0)).thenReturn(Type.int64()); + when(dmlReturningResultSet.getLong(0)).thenReturn(1L); + + StatementResult dmlReturningResult = mock(StatementResult.class); + when(dmlReturningResult.getResultType()).thenReturn(ResultType.RESULT_SET); + when(dmlReturningResult.getResultSet()).thenReturn(dmlReturningResultSet); + when(spanner.execute(com.google.cloud.spanner.Statement.of(DML_RETURNING))).thenReturn(dmlReturningResult); + StatementResult ddlResult = mock(StatementResult.class); when(ddlResult.getResultType()).thenReturn(ResultType.NO_RESULT); when(spanner.execute(com.google.cloud.spanner.Statement.of(DDL))).thenReturn(ddlResult); @@ -96,6 +124,8 @@ private JdbcStatement createStatement() throws SQLException { when(spanner.executeQuery(com.google.cloud.spanner.Statement.of(UPDATE))) .thenThrow( SpannerExceptionFactory.newSpannerException(ErrorCode.INVALID_ARGUMENT, "not a query")); + when(spanner.executeQuery(com.google.cloud.spanner.Statement.of(DML_RETURNING))) + .thenReturn(dmlReturningResultSet); when(spanner.executeQuery(com.google.cloud.spanner.Statement.of(DDL))) .thenThrow( SpannerExceptionFactory.newSpannerException(ErrorCode.INVALID_ARGUMENT, "not a query")); @@ -109,6 +139,10 @@ private JdbcStatement createStatement() throws SQLException { .thenThrow( SpannerExceptionFactory.newSpannerException( ErrorCode.INVALID_ARGUMENT, "not an update")); + when(spanner.executeUpdate(com.google.cloud.spanner.Statement.of(DML_RETURNING))) + .thenThrow( + SpannerExceptionFactory.newSpannerException( + ErrorCode.FAILED_PRECONDITION, "cannot execute dml returning over executeUpdate")); when(spanner.executeBatchUpdate(anyList())) .thenAnswer( @@ -219,6 +253,20 @@ public void testExecuteWithDdlStatement() throws SQLException { assertThat(statement.getUpdateCount()).isEqualTo(JdbcConstants.STATEMENT_NO_RESULT); } + @Test + public void testExecuteWithDmlReturningStatement() throws SQLException { + Statement statement = createStatement(); + boolean res = statement.execute(getDmlReturningString()); + assertThat(res).isTrue(); + assertThat(statement.getUpdateCount()).isEqualTo(JdbcConstants.STATEMENT_RESULT_SET); + try (ResultSet rs = statement.getResultSet()) { + assertNotNull(rs); + assertTrue(rs.next()); + assertEquals(rs.getLong(1), 1L); + assertFalse(rs.next()); + } + } + @Test public void testExecuteWithGeneratedKeys() throws SQLException { Statement statement = createStatement(); @@ -257,6 +305,17 @@ public void testExecuteQueryWithUpdateStatement() { } } + @Test + public void testExecuteQueryWithDmlReturningStatement() throws SQLException { + Statement statement = createStatement(); + try (ResultSet rs = statement.executeQuery(getDmlReturningString())) { + assertNotNull(rs); + assertTrue(rs.next()); + assertEquals(rs.getLong(1), 1L); + assertFalse(rs.next()); + } + } + @Test public void testExecuteQueryWithDdlStatement() { try { @@ -353,12 +412,27 @@ public void testExecuteUpdateWithSelectStatement() { } catch (SQLException e) { assertThat( JdbcExceptionMatcher.matchCodeAndMessage( - Code.INVALID_ARGUMENT, "The statement is not an update or DDL statement") + Code.INVALID_ARGUMENT, "The statement is not a normal update or DDL statement") .matches(e)) .isTrue(); } } + @Test + public void testExecuteUpdateWithDmlReturningStatement() { + try { + Statement statement = createStatement(); + statement.executeUpdate(getDmlReturningString()); + fail("missing expected exception"); + } catch (SQLException e) { + assertThat( + JdbcExceptionMatcher.matchCodeAndMessage( + Code.INVALID_ARGUMENT, "The statement is not a normal update or DDL statement") + .matches(e)) + .isTrue(); + } + } + @Test public void testExecuteUpdateWithDdlStatement() throws SQLException { Statement statement = createStatement(); @@ -433,7 +507,8 @@ public void testDmlBatch() throws SQLException { statement.addBatch("INSERT INTO FOO (ID, NAME) VALUES (1, 'TEST')"); statement.addBatch("INSERT INTO FOO (ID, NAME) VALUES (2, 'TEST')"); statement.addBatch("INSERT INTO FOO (ID, NAME) VALUES (3, 'TEST')"); - assertThat(statement.executeBatch()).asList().containsExactly(1, 1, 1); + statement.addBatch(getDmlReturningString()); + assertThat(statement.executeBatch()).asList().containsExactly(1, 1, 1, 1); } } } diff --git a/src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcPreparedStatementTest.java b/src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcPreparedStatementTest.java index c473b6e9..8b45a609 100644 --- a/src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcPreparedStatementTest.java +++ b/src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcPreparedStatementTest.java @@ -22,19 +22,27 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.junit.Assume.assumeFalse; +import com.google.api.client.util.IOUtils; import com.google.cloud.ByteArray; import com.google.cloud.spanner.Database; import com.google.cloud.spanner.Dialect; +import com.google.cloud.spanner.ErrorCode; import com.google.cloud.spanner.ParallelIntegrationTest; +import com.google.cloud.spanner.SpannerException; import com.google.cloud.spanner.Value; +import com.google.cloud.spanner.jdbc.JdbcSqlException; +import com.google.cloud.spanner.jdbc.JdbcSqlExceptionFactory.JdbcSqlExceptionImpl; import com.google.cloud.spanner.jdbc.JsonType; import com.google.cloud.spanner.testing.EmulatorSpannerHelper; import com.google.common.base.Strings; import com.google.common.io.BaseEncoding; +import com.google.common.io.CharStreams; import java.io.IOException; import java.io.InputStream; import java.io.StringReader; @@ -262,6 +270,24 @@ private void setPreparedStatement(Connection connection, PreparedStatement ps, D ps.setArray(6, connection.createArrayOf("INT64", this.ticketPrices)); } } + + private void assertEqualsFields(Connection connection, ResultSet rs, Dialect dialect) + throws SQLException { + assertEquals(rs.getLong(1), this.venueId); + assertEquals(rs.getLong(2), this.singerId); + if (dialect == Dialect.POSTGRESQL) { + assertEquals(rs.getString(3), this.concertDate.toString()); + assertEquals(rs.getString(4), this.beginTime.toString()); + assertEquals(rs.getString(5), this.endTime.toString()); + } else { + assertEquals(rs.getDate(3), this.concertDate); + assertEquals(rs.getTimestamp(4), this.beginTime); + assertEquals(rs.getTimestamp(5), this.endTime); + assertArrayEquals( + (Object[]) rs.getArray(6).getArray(), + (Object[]) connection.createArrayOf("INT64", this.ticketPrices).getArray()); + } + } } private static Date parseDate(String value) { @@ -332,6 +358,34 @@ private String getConcertsInsertQuery(Dialect dialect) { return "INSERT INTO Concerts (VenueId, SingerId, ConcertDate, BeginTime, EndTime, TicketPrices) VALUES (?,?,?,?,?,?);"; } + private String getConcertsInsertReturningQuery(Dialect dialect) { + if (dialect == Dialect.POSTGRESQL) { + return "INSERT INTO Concerts (VenueId, SingerId, ConcertDate, BeginTime, EndTime) VALUES (?,?,?,?,?) RETURNING *;"; + } + return "INSERT INTO Concerts (VenueId, SingerId, ConcertDate, BeginTime, EndTime, TicketPrices) VALUES (?,?,?,?,?,?) THEN RETURN *;"; + } + + private String getSingersInsertReturningQuery(Dialect dialect) { + if (dialect == Dialect.POSTGRESQL) { + return "INSERT INTO Singers (SingerId, FirstName, LastName, SingerInfo, BirthDate) values (?,?,?,?,?) RETURNING *"; + } + return "INSERT INTO Singers (SingerId, FirstName, LastName, SingerInfo, BirthDate) values (?,?,?,?,?) THEN RETURN *"; + } + + private String getAlbumsInsertReturningQuery(Dialect dialect) { + if (dialect == Dialect.POSTGRESQL) { + return "INSERT INTO Albums (SingerId, AlbumId, AlbumTitle, MarketingBudget) VALUES (?,?,?,?) RETURNING *"; + } + return "INSERT INTO Albums (SingerId, AlbumId, AlbumTitle, MarketingBudget) VALUES (?,?,?,?) THEN RETURN *"; + } + + private String getSongsInsertReturningQuery(Dialect dialect) { + if (dialect == Dialect.POSTGRESQL) { + return "INSERT INTO Songs (SingerId, AlbumId, TrackId, SongName, Duration, SongGenre) VALUES (?,?,?,?,?,?) RETURNING *;"; + } + return "INSERT INTO Songs (SingerId, AlbumId, TrackId, SongName, Duration, SongGenre) VALUES (?,?,?,?,?,?) THEN RETURN *;"; + } + private int getConcertExpectedParamCount(Dialect dialect) { if (dialect == Dialect.POSTGRESQL) { return 5; @@ -1104,6 +1158,89 @@ private void assertDefaultParameterMetaData(ParameterMetaData pmd, int expectedP } } + @Test + public void test12_InsertReturningTestData() throws SQLException { + try (Connection connection = createConnection(env, database)) { + connection.setAutoCommit(false); + try (PreparedStatement ps = + connection.prepareStatement(getSingersInsertReturningQuery(dialect.dialect))) { + assertDefaultParameterMetaData(ps.getParameterMetaData(), 5); + for (Singer singer : createSingers()) { + singer.setPreparedStatement(ps, getDialect()); + assertInsertSingerParameterMetadata(ps.getParameterMetaData()); + ps.addBatch(); + // check that adding the current params to a batch will not reset the metadata + assertInsertSingerParameterMetadata(ps.getParameterMetaData()); + } + int[] results = ps.executeBatch(); + for (int res : results) { + assertEquals(1, res); + } + } + try (PreparedStatement ps = + connection.prepareStatement(getAlbumsInsertReturningQuery(dialect.dialect))) { + assertDefaultParameterMetaData(ps.getParameterMetaData(), 4); + for (Album album : createAlbums()) { + ps.setLong(1, album.singerId); + ps.setLong(2, album.albumId); + ps.setString(3, album.albumTitle); + ps.setLong(4, album.marketingBudget); + assertInsertAlbumParameterMetadata(ps.getParameterMetaData()); + ResultSet rs = ps.executeQuery(); + rs.next(); + assertEquals(rs.getLong(1), album.singerId); + assertEquals(rs.getLong(2), album.albumId); + assertEquals(rs.getString(3), album.albumTitle); + assertEquals(rs.getLong(4), album.marketingBudget); + // check that calling executeQuery will not reset the metadata + assertInsertAlbumParameterMetadata(ps.getParameterMetaData()); + } + } + try (PreparedStatement ps = + connection.prepareStatement(getSongsInsertReturningQuery(dialect.dialect))) { + assertDefaultParameterMetaData(ps.getParameterMetaData(), 6); + for (Song song : createSongs()) { + ps.setByte(1, (byte) song.singerId); + ps.setInt(2, (int) song.albumId); + ps.setShort(3, (short) song.songId); + ps.setNString(4, song.songName); + ps.setLong(5, song.duration); + ps.setCharacterStream(6, new StringReader(song.songGenre)); + assertInsertSongParameterMetadata(ps.getParameterMetaData()); + ResultSet rs = ps.executeQuery(); + rs.next(); + assertEquals(rs.getByte(1), (byte) song.singerId); + assertEquals(rs.getInt(2), (int) song.albumId); + assertEquals(rs.getShort(3), (short) song.songId); + assertEquals(rs.getNString(4), song.songName); + assertEquals(rs.getLong(5), song.duration); + assertEquals( + CharStreams.toString(rs.getCharacterStream(6)), + CharStreams.toString(new StringReader(song.songGenre))); + // check that calling executeQuery will not reset the metadata + assertInsertSongParameterMetadata(ps.getParameterMetaData()); + } + } catch (IOException e) { + // ignore exception. + } + try (PreparedStatement ps = + connection.prepareStatement(getConcertsInsertReturningQuery(dialect.dialect))) { + assertDefaultParameterMetaData( + ps.getParameterMetaData(), getConcertExpectedParamCount(dialect.dialect)); + for (Concert concert : createConcerts()) { + concert.setPreparedStatement(connection, ps, getDialect()); + assertInsertConcertParameterMetadata(ps.getParameterMetaData()); + ResultSet rs = ps.executeQuery(); + rs.next(); + concert.assertEqualsFields(connection, rs, dialect.dialect); + // check that calling executeUpdate will not reset the meta data + assertInsertConcertParameterMetadata(ps.getParameterMetaData()); + } + } + connection.commit(); + } + } + private List readValuesFromFile(String filename) { StringBuilder builder = new StringBuilder(); try (InputStream stream = ITJdbcPreparedStatementTest.class.getResourceAsStream(filename)) { From f5eb91bbec3f8de43b15b88f92add32f0f977c49 Mon Sep 17 00:00:00 2001 From: Rajat Bhatta Date: Mon, 22 Aug 2022 17:21:08 +0000 Subject: [PATCH 02/10] fix: linting --- .../cloud/spanner/jdbc/JdbcStatementTest.java | 13 ++++++++----- .../jdbc/it/ITJdbcPreparedStatementTest.java | 7 ------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/test/java/com/google/cloud/spanner/jdbc/JdbcStatementTest.java b/src/test/java/com/google/cloud/spanner/jdbc/JdbcStatementTest.java index bf8d8500..cf995b0e 100644 --- a/src/test/java/com/google/cloud/spanner/jdbc/JdbcStatementTest.java +++ b/src/test/java/com/google/cloud/spanner/jdbc/JdbcStatementTest.java @@ -114,7 +114,8 @@ private JdbcStatement createStatement() throws SQLException { StatementResult dmlReturningResult = mock(StatementResult.class); when(dmlReturningResult.getResultType()).thenReturn(ResultType.RESULT_SET); when(dmlReturningResult.getResultSet()).thenReturn(dmlReturningResultSet); - when(spanner.execute(com.google.cloud.spanner.Statement.of(DML_RETURNING))).thenReturn(dmlReturningResult); + when(spanner.execute(com.google.cloud.spanner.Statement.of(DML_RETURNING))) + .thenReturn(dmlReturningResult); StatementResult ddlResult = mock(StatementResult.class); when(ddlResult.getResultType()).thenReturn(ResultType.NO_RESULT); @@ -412,7 +413,8 @@ public void testExecuteUpdateWithSelectStatement() { } catch (SQLException e) { assertThat( JdbcExceptionMatcher.matchCodeAndMessage( - Code.INVALID_ARGUMENT, "The statement is not a normal update or DDL statement") + Code.INVALID_ARGUMENT, + "The statement is not a normal update or DDL statement") .matches(e)) .isTrue(); } @@ -426,9 +428,10 @@ public void testExecuteUpdateWithDmlReturningStatement() { fail("missing expected exception"); } catch (SQLException e) { assertThat( - JdbcExceptionMatcher.matchCodeAndMessage( - Code.INVALID_ARGUMENT, "The statement is not a normal update or DDL statement") - .matches(e)) + JdbcExceptionMatcher.matchCodeAndMessage( + Code.INVALID_ARGUMENT, + "The statement is not a normal update or DDL statement") + .matches(e)) .isTrue(); } } diff --git a/src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcPreparedStatementTest.java b/src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcPreparedStatementTest.java index 8b45a609..459ef5a2 100644 --- a/src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcPreparedStatementTest.java +++ b/src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcPreparedStatementTest.java @@ -22,22 +22,15 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.junit.Assume.assumeFalse; -import com.google.api.client.util.IOUtils; import com.google.cloud.ByteArray; import com.google.cloud.spanner.Database; import com.google.cloud.spanner.Dialect; -import com.google.cloud.spanner.ErrorCode; import com.google.cloud.spanner.ParallelIntegrationTest; -import com.google.cloud.spanner.SpannerException; import com.google.cloud.spanner.Value; -import com.google.cloud.spanner.jdbc.JdbcSqlException; -import com.google.cloud.spanner.jdbc.JdbcSqlExceptionFactory.JdbcSqlExceptionImpl; import com.google.cloud.spanner.jdbc.JsonType; import com.google.cloud.spanner.testing.EmulatorSpannerHelper; import com.google.common.base.Strings; From 76cb6c277f0cf99f30d62c8387f5af45bdfcfc08 Mon Sep 17 00:00:00 2001 From: Rajat Bhatta Date: Mon, 22 Aug 2022 17:26:28 +0000 Subject: [PATCH 03/10] feat: changes --- .../java/com/google/cloud/spanner/jdbc/JdbcStatementTest.java | 3 +-- .../cloud/spanner/jdbc/it/ITJdbcPreparedStatementTest.java | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/test/java/com/google/cloud/spanner/jdbc/JdbcStatementTest.java b/src/test/java/com/google/cloud/spanner/jdbc/JdbcStatementTest.java index cf995b0e..39b411dc 100644 --- a/src/test/java/com/google/cloud/spanner/jdbc/JdbcStatementTest.java +++ b/src/test/java/com/google/cloud/spanner/jdbc/JdbcStatementTest.java @@ -71,9 +71,8 @@ public static Object[] data() { private String getDmlReturningString() { if (dialect == Dialect.GOOGLE_STANDARD_SQL) { return DML_RETURNING_GSQL; - } else { - return DML_RETURNING_PG; } + return DML_RETURNING_PG; } @SuppressWarnings("unchecked") diff --git a/src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcPreparedStatementTest.java b/src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcPreparedStatementTest.java index 459ef5a2..7173a227 100644 --- a/src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcPreparedStatementTest.java +++ b/src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcPreparedStatementTest.java @@ -1226,7 +1226,7 @@ public void test12_InsertReturningTestData() throws SQLException { ResultSet rs = ps.executeQuery(); rs.next(); concert.assertEqualsFields(connection, rs, dialect.dialect); - // check that calling executeUpdate will not reset the meta data + // check that calling executeQuery will not reset the meta data assertInsertConcertParameterMetadata(ps.getParameterMetaData()); } } From 9181b167838ed76bb110dbf8601d4606e0bbdc98 Mon Sep 17 00:00:00 2001 From: Rajat Bhatta Date: Mon, 22 Aug 2022 17:30:26 +0000 Subject: [PATCH 04/10] feat: change variable, method name --- .../cloud/spanner/jdbc/JdbcStatementTest.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/test/java/com/google/cloud/spanner/jdbc/JdbcStatementTest.java b/src/test/java/com/google/cloud/spanner/jdbc/JdbcStatementTest.java index 39b411dc..7b79389c 100644 --- a/src/test/java/com/google/cloud/spanner/jdbc/JdbcStatementTest.java +++ b/src/test/java/com/google/cloud/spanner/jdbc/JdbcStatementTest.java @@ -68,7 +68,7 @@ public static Object[] data() { return Dialect.values(); } - private String getDmlReturningString() { + private String getDmlReturningSql() { if (dialect == Dialect.GOOGLE_STANDARD_SQL) { return DML_RETURNING_GSQL; } @@ -80,7 +80,7 @@ private JdbcStatement createStatement() throws SQLException { Connection spanner = mock(Connection.class); when(spanner.getDialect()).thenReturn(dialect); - final String DML_RETURNING = getDmlReturningString(); + final String DML_RETURNING_SQL = getDmlReturningSql(); com.google.cloud.spanner.ResultSet resultSet = mock(com.google.cloud.spanner.ResultSet.class); when(resultSet.next()).thenReturn(true, false); @@ -113,7 +113,7 @@ private JdbcStatement createStatement() throws SQLException { StatementResult dmlReturningResult = mock(StatementResult.class); when(dmlReturningResult.getResultType()).thenReturn(ResultType.RESULT_SET); when(dmlReturningResult.getResultSet()).thenReturn(dmlReturningResultSet); - when(spanner.execute(com.google.cloud.spanner.Statement.of(DML_RETURNING))) + when(spanner.execute(com.google.cloud.spanner.Statement.of(DML_RETURNING_SQL))) .thenReturn(dmlReturningResult); StatementResult ddlResult = mock(StatementResult.class); @@ -124,7 +124,7 @@ private JdbcStatement createStatement() throws SQLException { when(spanner.executeQuery(com.google.cloud.spanner.Statement.of(UPDATE))) .thenThrow( SpannerExceptionFactory.newSpannerException(ErrorCode.INVALID_ARGUMENT, "not a query")); - when(spanner.executeQuery(com.google.cloud.spanner.Statement.of(DML_RETURNING))) + when(spanner.executeQuery(com.google.cloud.spanner.Statement.of(DML_RETURNING_SQL))) .thenReturn(dmlReturningResultSet); when(spanner.executeQuery(com.google.cloud.spanner.Statement.of(DDL))) .thenThrow( @@ -139,7 +139,7 @@ private JdbcStatement createStatement() throws SQLException { .thenThrow( SpannerExceptionFactory.newSpannerException( ErrorCode.INVALID_ARGUMENT, "not an update")); - when(spanner.executeUpdate(com.google.cloud.spanner.Statement.of(DML_RETURNING))) + when(spanner.executeUpdate(com.google.cloud.spanner.Statement.of(DML_RETURNING_SQL))) .thenThrow( SpannerExceptionFactory.newSpannerException( ErrorCode.FAILED_PRECONDITION, "cannot execute dml returning over executeUpdate")); @@ -256,7 +256,7 @@ public void testExecuteWithDdlStatement() throws SQLException { @Test public void testExecuteWithDmlReturningStatement() throws SQLException { Statement statement = createStatement(); - boolean res = statement.execute(getDmlReturningString()); + boolean res = statement.execute(getDmlReturningSql()); assertThat(res).isTrue(); assertThat(statement.getUpdateCount()).isEqualTo(JdbcConstants.STATEMENT_RESULT_SET); try (ResultSet rs = statement.getResultSet()) { @@ -308,7 +308,7 @@ public void testExecuteQueryWithUpdateStatement() { @Test public void testExecuteQueryWithDmlReturningStatement() throws SQLException { Statement statement = createStatement(); - try (ResultSet rs = statement.executeQuery(getDmlReturningString())) { + try (ResultSet rs = statement.executeQuery(getDmlReturningSql())) { assertNotNull(rs); assertTrue(rs.next()); assertEquals(rs.getLong(1), 1L); @@ -423,7 +423,7 @@ public void testExecuteUpdateWithSelectStatement() { public void testExecuteUpdateWithDmlReturningStatement() { try { Statement statement = createStatement(); - statement.executeUpdate(getDmlReturningString()); + statement.executeUpdate(getDmlReturningSql()); fail("missing expected exception"); } catch (SQLException e) { assertThat( @@ -509,7 +509,7 @@ public void testDmlBatch() throws SQLException { statement.addBatch("INSERT INTO FOO (ID, NAME) VALUES (1, 'TEST')"); statement.addBatch("INSERT INTO FOO (ID, NAME) VALUES (2, 'TEST')"); statement.addBatch("INSERT INTO FOO (ID, NAME) VALUES (3, 'TEST')"); - statement.addBatch(getDmlReturningString()); + statement.addBatch(getDmlReturningSql()); assertThat(statement.executeBatch()).asList().containsExactly(1, 1, 1, 1); } } From 7d7da949a2821d5f7f486fb09f7ded30a7615abf Mon Sep 17 00:00:00 2001 From: Rajat Bhatta Date: Mon, 22 Aug 2022 17:36:18 +0000 Subject: [PATCH 05/10] feat: ignore returning test for emulator --- .../cloud/spanner/jdbc/it/ITJdbcPreparedStatementTest.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcPreparedStatementTest.java b/src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcPreparedStatementTest.java index 7173a227..7d877ce1 100644 --- a/src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcPreparedStatementTest.java +++ b/src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcPreparedStatementTest.java @@ -1153,6 +1153,9 @@ private void assertDefaultParameterMetaData(ParameterMetaData pmd, int expectedP @Test public void test12_InsertReturningTestData() throws SQLException { + assumeFalse( + "Emulator does not support DML with returning clause", + EmulatorSpannerHelper.isUsingEmulator()); try (Connection connection = createConnection(env, database)) { connection.setAutoCommit(false); try (PreparedStatement ps = From 19f022a69cd396477388aade01540967338bb798 Mon Sep 17 00:00:00 2001 From: Rajat Bhatta Date: Tue, 23 Aug 2022 11:10:25 +0000 Subject: [PATCH 06/10] fix: incorporate review comments --- .../jdbc/it/ITJdbcPreparedStatementTest.java | 41 ++++++++++--------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcPreparedStatementTest.java b/src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcPreparedStatementTest.java index 7d877ce1..4ffd53fd 100644 --- a/src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcPreparedStatementTest.java +++ b/src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcPreparedStatementTest.java @@ -1182,12 +1182,13 @@ public void test12_InsertReturningTestData() throws SQLException { ps.setString(3, album.albumTitle); ps.setLong(4, album.marketingBudget); assertInsertAlbumParameterMetadata(ps.getParameterMetaData()); - ResultSet rs = ps.executeQuery(); - rs.next(); - assertEquals(rs.getLong(1), album.singerId); - assertEquals(rs.getLong(2), album.albumId); - assertEquals(rs.getString(3), album.albumTitle); - assertEquals(rs.getLong(4), album.marketingBudget); + try (ResultSet rs = ps.executeQuery()) { + rs.next(); + assertEquals(rs.getLong(1), album.singerId); + assertEquals(rs.getLong(2), album.albumId); + assertEquals(rs.getString(3), album.albumTitle); + assertEquals(rs.getLong(4), album.marketingBudget); + } // check that calling executeQuery will not reset the metadata assertInsertAlbumParameterMetadata(ps.getParameterMetaData()); } @@ -1203,16 +1204,17 @@ public void test12_InsertReturningTestData() throws SQLException { ps.setLong(5, song.duration); ps.setCharacterStream(6, new StringReader(song.songGenre)); assertInsertSongParameterMetadata(ps.getParameterMetaData()); - ResultSet rs = ps.executeQuery(); - rs.next(); - assertEquals(rs.getByte(1), (byte) song.singerId); - assertEquals(rs.getInt(2), (int) song.albumId); - assertEquals(rs.getShort(3), (short) song.songId); - assertEquals(rs.getNString(4), song.songName); - assertEquals(rs.getLong(5), song.duration); - assertEquals( - CharStreams.toString(rs.getCharacterStream(6)), - CharStreams.toString(new StringReader(song.songGenre))); + try (ResultSet rs = ps.executeQuery()) { + rs.next(); + assertEquals(rs.getByte(1), (byte) song.singerId); + assertEquals(rs.getInt(2), (int) song.albumId); + assertEquals(rs.getShort(3), (short) song.songId); + assertEquals(rs.getNString(4), song.songName); + assertEquals(rs.getLong(5), song.duration); + assertEquals( + CharStreams.toString(rs.getCharacterStream(6)), + CharStreams.toString(new StringReader(song.songGenre))); + } // check that calling executeQuery will not reset the metadata assertInsertSongParameterMetadata(ps.getParameterMetaData()); } @@ -1226,9 +1228,10 @@ public void test12_InsertReturningTestData() throws SQLException { for (Concert concert : createConcerts()) { concert.setPreparedStatement(connection, ps, getDialect()); assertInsertConcertParameterMetadata(ps.getParameterMetaData()); - ResultSet rs = ps.executeQuery(); - rs.next(); - concert.assertEqualsFields(connection, rs, dialect.dialect); + try (ResultSet rs = ps.executeQuery()) { + rs.next(); + concert.assertEqualsFields(connection, rs, dialect.dialect); + } // check that calling executeQuery will not reset the meta data assertInsertConcertParameterMetadata(ps.getParameterMetaData()); } From 4bcb9f2b1ae679cbbd177bbc0b2e4dd6adc09674 Mon Sep 17 00:00:00 2001 From: Rajat Bhatta Date: Tue, 23 Aug 2022 15:59:31 +0000 Subject: [PATCH 07/10] test: add separete tests for DML Returning in JdbcStatementTest --- .../cloud/spanner/jdbc/JdbcStatementTest.java | 31 +++++++++++++------ 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/src/test/java/com/google/cloud/spanner/jdbc/JdbcStatementTest.java b/src/test/java/com/google/cloud/spanner/jdbc/JdbcStatementTest.java index 7b79389c..872690cd 100644 --- a/src/test/java/com/google/cloud/spanner/jdbc/JdbcStatementTest.java +++ b/src/test/java/com/google/cloud/spanner/jdbc/JdbcStatementTest.java @@ -17,9 +17,11 @@ package com.google.cloud.spanner.jdbc; import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.Mockito.anyList; @@ -423,15 +425,14 @@ public void testExecuteUpdateWithSelectStatement() { public void testExecuteUpdateWithDmlReturningStatement() { try { Statement statement = createStatement(); - statement.executeUpdate(getDmlReturningSql()); - fail("missing expected exception"); + SQLException e = + assertThrows(SQLException.class, () -> statement.executeUpdate(getDmlReturningSql())); + assertTrue( + JdbcExceptionMatcher.matchCodeAndMessage( + Code.INVALID_ARGUMENT, "The statement is not a normal update or DDL statement") + .matches(e)); } catch (SQLException e) { - assertThat( - JdbcExceptionMatcher.matchCodeAndMessage( - Code.INVALID_ARGUMENT, - "The statement is not a normal update or DDL statement") - .matches(e)) - .isTrue(); + // ignore exception. } } @@ -509,12 +510,24 @@ public void testDmlBatch() throws SQLException { statement.addBatch("INSERT INTO FOO (ID, NAME) VALUES (1, 'TEST')"); statement.addBatch("INSERT INTO FOO (ID, NAME) VALUES (2, 'TEST')"); statement.addBatch("INSERT INTO FOO (ID, NAME) VALUES (3, 'TEST')"); - statement.addBatch(getDmlReturningSql()); assertThat(statement.executeBatch()).asList().containsExactly(1, 1, 1, 1); } } } + @Test + public void testDmlBatchWithDmlReturning() throws SQLException { + try (Statement statement = createStatement()) { + // Verify that multiple batches can be executed on the same statement. + for (int i = 0; i < 2; i++) { + statement.addBatch(getDmlReturningSql()); + statement.addBatch(getDmlReturningSql()); + statement.addBatch(getDmlReturningSql()); + assertArrayEquals(statement.executeBatch(), new int[] {1, 1, 1}); + } + } + } + @Test public void testLargeDmlBatch() throws SQLException { try (Statement statement = createStatement()) { From 1c1217cdf748ac4cd54578e983fab61019ac10a8 Mon Sep 17 00:00:00 2001 From: Rajat Bhatta Date: Fri, 26 Aug 2022 09:24:48 +0000 Subject: [PATCH 08/10] feat: change error message --- .../com/google/cloud/spanner/jdbc/JdbcStatement.java | 2 +- .../google/cloud/spanner/jdbc/JdbcStatementTest.java | 11 ++++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/google/cloud/spanner/jdbc/JdbcStatement.java b/src/main/java/com/google/cloud/spanner/jdbc/JdbcStatement.java index 42ef98c0..051ed9d6 100644 --- a/src/main/java/com/google/cloud/spanner/jdbc/JdbcStatement.java +++ b/src/main/java/com/google/cloud/spanner/jdbc/JdbcStatement.java @@ -87,7 +87,7 @@ public long executeLargeUpdate(String sql) throws SQLException { switch (result.getResultType()) { case RESULT_SET: throw JdbcSqlExceptionFactory.of( - "The statement is not a normal update or DDL statement", Code.INVALID_ARGUMENT); + "The statement is not a non-returning DML or DDL statement", Code.INVALID_ARGUMENT); case UPDATE_COUNT: return result.getUpdateCount(); case NO_RESULT: diff --git a/src/test/java/com/google/cloud/spanner/jdbc/JdbcStatementTest.java b/src/test/java/com/google/cloud/spanner/jdbc/JdbcStatementTest.java index 872690cd..c7b6a6cb 100644 --- a/src/test/java/com/google/cloud/spanner/jdbc/JdbcStatementTest.java +++ b/src/test/java/com/google/cloud/spanner/jdbc/JdbcStatementTest.java @@ -259,8 +259,8 @@ public void testExecuteWithDdlStatement() throws SQLException { public void testExecuteWithDmlReturningStatement() throws SQLException { Statement statement = createStatement(); boolean res = statement.execute(getDmlReturningSql()); - assertThat(res).isTrue(); - assertThat(statement.getUpdateCount()).isEqualTo(JdbcConstants.STATEMENT_RESULT_SET); + assertTrue(res); + assertEquals(statement.getUpdateCount(), JdbcConstants.STATEMENT_RESULT_SET); try (ResultSet rs = statement.getResultSet()) { assertNotNull(rs); assertTrue(rs.next()); @@ -415,7 +415,7 @@ public void testExecuteUpdateWithSelectStatement() { assertThat( JdbcExceptionMatcher.matchCodeAndMessage( Code.INVALID_ARGUMENT, - "The statement is not a normal update or DDL statement") + "The statement is not a non-returning DML or DDL statement") .matches(e)) .isTrue(); } @@ -429,7 +429,8 @@ public void testExecuteUpdateWithDmlReturningStatement() { assertThrows(SQLException.class, () -> statement.executeUpdate(getDmlReturningSql())); assertTrue( JdbcExceptionMatcher.matchCodeAndMessage( - Code.INVALID_ARGUMENT, "The statement is not a normal update or DDL statement") + Code.INVALID_ARGUMENT, + "The statement is not a non-returning DML or DDL statement") .matches(e)); } catch (SQLException e) { // ignore exception. @@ -510,7 +511,7 @@ public void testDmlBatch() throws SQLException { statement.addBatch("INSERT INTO FOO (ID, NAME) VALUES (1, 'TEST')"); statement.addBatch("INSERT INTO FOO (ID, NAME) VALUES (2, 'TEST')"); statement.addBatch("INSERT INTO FOO (ID, NAME) VALUES (3, 'TEST')"); - assertThat(statement.executeBatch()).asList().containsExactly(1, 1, 1, 1); + assertThat(statement.executeBatch()).asList().containsExactly(1, 1, 1); } } } From 03da6fdce76373d95182618ff71a552bc78ec699 Mon Sep 17 00:00:00 2001 From: Rajat Bhatta Date: Mon, 21 Nov 2022 11:10:41 +0000 Subject: [PATCH 09/10] fix: delete existing rows before returning test --- .../spanner/jdbc/it/ITJdbcPreparedStatementTest.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcPreparedStatementTest.java b/src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcPreparedStatementTest.java index 17f872b7..a1a67cb3 100644 --- a/src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcPreparedStatementTest.java +++ b/src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcPreparedStatementTest.java @@ -1204,6 +1204,14 @@ public void test12_InsertReturningTestData() throws SQLException { EmulatorSpannerHelper.isUsingEmulator()); try (Connection connection = createConnection(env, database)) { connection.setAutoCommit(false); + // Delete existing rows from tables populated by other tests, + // so that this test can populate rows from scratch. + Statement deleteStatements = connection.createStatement(); + deleteStatements.addBatch("DELETE FROM Singers WHERE TRUE"); + deleteStatements.addBatch("DELETE FROM Albums WHERE TRUE"); + deleteStatements.addBatch("DELETE FROM Songs WHERE TRUE"); + deleteStatements.addBatch("DELETE FROM Concerts WHERE TRUE"); + deleteStatements.executeBatch(); try (PreparedStatement ps = connection.prepareStatement(getSingersInsertReturningQuery(dialect.dialect))) { assertDefaultParameterMetaData(ps.getParameterMetaData(), 5); From 7ce66c447ddc80292098e906e50fb57071412697 Mon Sep 17 00:00:00 2001 From: Rajat Bhatta Date: Mon, 21 Nov 2022 11:32:44 +0000 Subject: [PATCH 10/10] fix: order of delete from tables --- .../cloud/spanner/jdbc/it/ITJdbcPreparedStatementTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcPreparedStatementTest.java b/src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcPreparedStatementTest.java index a1a67cb3..c1d84169 100644 --- a/src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcPreparedStatementTest.java +++ b/src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcPreparedStatementTest.java @@ -1207,10 +1207,10 @@ public void test12_InsertReturningTestData() throws SQLException { // Delete existing rows from tables populated by other tests, // so that this test can populate rows from scratch. Statement deleteStatements = connection.createStatement(); - deleteStatements.addBatch("DELETE FROM Singers WHERE TRUE"); - deleteStatements.addBatch("DELETE FROM Albums WHERE TRUE"); - deleteStatements.addBatch("DELETE FROM Songs WHERE TRUE"); deleteStatements.addBatch("DELETE FROM Concerts WHERE TRUE"); + deleteStatements.addBatch("DELETE FROM Songs WHERE TRUE"); + deleteStatements.addBatch("DELETE FROM Albums WHERE TRUE"); + deleteStatements.addBatch("DELETE FROM Singers WHERE TRUE"); deleteStatements.executeBatch(); try (PreparedStatement ps = connection.prepareStatement(getSingersInsertReturningQuery(dialect.dialect))) {