From bc5b691ecb718f8b358dc7783b1583c44f4d89b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Fri, 28 Jul 2023 16:14:29 +0200 Subject: [PATCH 1/2] chore: refactor client side statements to accept the entire parsed statement Refactor the internal interface of client-side statements so these receive the entire parsed statement, including any query parameters in the statement. This allows us to create client-side statements that actually use the query parameters that have been specified by the user. --- .../connection/ClientSideStatement.java | 3 +- .../ClientSideStatementExecutor.java | 6 ++- .../ClientSideStatementExplainExecutor.java | 6 ++- .../connection/ClientSideStatementImpl.java | 8 +-- .../ClientSideStatementNoParamExecutor.java | 3 +- .../ClientSideStatementPgBeginExecutor.java | 6 ++- .../ClientSideStatementSetExecutor.java | 6 ++- .../spanner/connection/ConnectionImpl.java | 8 +-- .../connection/BeginPgTransactionTest.java | 10 ++-- ...nnectionStatementWithNoParametersTest.java | 36 ++++++++----- ...nnectionStatementWithOneParameterTest.java | 50 ++++++++++--------- .../SetPgSessionCharacteristicsTest.java | 20 ++++---- 12 files changed, 94 insertions(+), 68 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ClientSideStatement.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ClientSideStatement.java index 08f14b146a2..521ba546073 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ClientSideStatement.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ClientSideStatement.java @@ -17,6 +17,7 @@ package com.google.cloud.spanner.connection; import com.google.cloud.spanner.ResultSet; +import com.google.cloud.spanner.connection.AbstractStatementParser.ParsedStatement; import com.google.cloud.spanner.connection.StatementResult.ClientSideStatementType; import java.util.List; @@ -63,5 +64,5 @@ interface ClientSideStatement { * needed for the execution of the {@link ClientSideStatement}. * @return the result of the execution of the statement. */ - StatementResult execute(ConnectionStatementExecutor executor, String statement); + StatementResult execute(ConnectionStatementExecutor executor, ParsedStatement statement); } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ClientSideStatementExecutor.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ClientSideStatementExecutor.java index 7ec09766697..dae773f606a 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ClientSideStatementExecutor.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ClientSideStatementExecutor.java @@ -16,6 +16,8 @@ package com.google.cloud.spanner.connection; +import com.google.cloud.spanner.connection.AbstractStatementParser.ParsedStatement; + /** * A {@link ClientSideStatementExecutor} is used to compile {@link ClientSideStatement}s from the * json source file, and to execute these against a {@link Connection} (through a {@link @@ -29,13 +31,13 @@ interface ClientSideStatementExecutor { * * @param connectionExecutor The {@link ConnectionStatementExecutor} to use to execute the * statement on a {@link Connection}. - * @param sql The sql statement that is executed. This can be used to parse any additional + * @param statement The statement that is executed. This can be used to parse any additional * arguments that might be needed for the execution of the {@link ClientSideStatementImpl}. * @return the result of the execution. * @throws Exception If an error occurs while executing the statement, for example if an invalid * argument has been specified in the sql statement, or if the statement is invalid for the * current state of the {@link Connection}. */ - StatementResult execute(ConnectionStatementExecutor connectionExecutor, String sql) + StatementResult execute(ConnectionStatementExecutor connectionExecutor, ParsedStatement statement) throws Exception; } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ClientSideStatementExplainExecutor.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ClientSideStatementExplainExecutor.java index 7ce201cb0bd..767d6917be6 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ClientSideStatementExplainExecutor.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ClientSideStatementExplainExecutor.java @@ -18,6 +18,7 @@ import com.google.cloud.spanner.ErrorCode; import com.google.cloud.spanner.SpannerExceptionFactory; +import com.google.cloud.spanner.connection.AbstractStatementParser.ParsedStatement; import com.google.cloud.spanner.connection.ClientSideStatementImpl.CompileException; import com.google.cloud.spanner.connection.ClientSideStatementValueConverters.ExplainCommandConverter; import com.google.common.collect.ImmutableSet; @@ -47,9 +48,10 @@ class ClientSideStatementExplainExecutor implements ClientSideStatementExecutor } @Override - public StatementResult execute(ConnectionStatementExecutor connection, String sql) + public StatementResult execute(ConnectionStatementExecutor connection, ParsedStatement statement) throws Exception { - return (StatementResult) method.invoke(connection, getParameterValue(sql)); + return (StatementResult) + method.invoke(connection, getParameterValue(statement.getSqlWithoutComments())); } String getParameterValue(String sql) { diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ClientSideStatementImpl.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ClientSideStatementImpl.java index 2ff39131baa..f9ecba6652a 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ClientSideStatementImpl.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ClientSideStatementImpl.java @@ -17,6 +17,7 @@ package com.google.cloud.spanner.connection; import com.google.cloud.spanner.SpannerException; +import com.google.cloud.spanner.connection.AbstractStatementParser.ParsedStatement; import com.google.cloud.spanner.connection.StatementResult.ClientSideStatementType; import com.google.cloud.spanner.connection.StatementResult.ResultType; import com.google.common.base.Preconditions; @@ -160,7 +161,8 @@ ClientSideStatementImpl compile() throws CompileException { } @Override - public StatementResult execute(ConnectionStatementExecutor connection, String statement) { + public StatementResult execute( + ConnectionStatementExecutor connection, ParsedStatement statement) { Preconditions.checkState(executor != null, "This statement has not been compiled"); try { return executor.execute(connection, statement); @@ -170,9 +172,9 @@ public StatementResult execute(ConnectionStatementExecutor connection, String st if (e.getCause() instanceof SpannerException) { throw (SpannerException) e.getCause(); } - throw new ExecuteException(e.getCause(), this, statement); + throw new ExecuteException(e.getCause(), this, statement.getStatement().getSql()); } catch (Exception e) { - throw new ExecuteException(e, this, statement); + throw new ExecuteException(e, this, statement.getStatement().getSql()); } } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ClientSideStatementNoParamExecutor.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ClientSideStatementNoParamExecutor.java index 6f5e683ce64..01da308c166 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ClientSideStatementNoParamExecutor.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ClientSideStatementNoParamExecutor.java @@ -16,6 +16,7 @@ package com.google.cloud.spanner.connection; +import com.google.cloud.spanner.connection.AbstractStatementParser.ParsedStatement; import com.google.cloud.spanner.connection.ClientSideStatementImpl.CompileException; import java.lang.reflect.Method; @@ -42,7 +43,7 @@ class ClientSideStatementNoParamExecutor implements ClientSideStatementExecutor } @Override - public StatementResult execute(ConnectionStatementExecutor connection, String statement) + public StatementResult execute(ConnectionStatementExecutor connection, ParsedStatement statement) throws Exception { return (StatementResult) method.invoke(connection); } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ClientSideStatementPgBeginExecutor.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ClientSideStatementPgBeginExecutor.java index 4b548803c11..c1d00d81b55 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ClientSideStatementPgBeginExecutor.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ClientSideStatementPgBeginExecutor.java @@ -18,6 +18,7 @@ import com.google.cloud.spanner.ErrorCode; import com.google.cloud.spanner.SpannerExceptionFactory; +import com.google.cloud.spanner.connection.AbstractStatementParser.ParsedStatement; import com.google.cloud.spanner.connection.ClientSideStatementImpl.CompileException; import com.google.cloud.spanner.connection.ClientSideStatementValueConverters.PgTransactionModeConverter; import java.lang.reflect.Method; @@ -42,9 +43,10 @@ class ClientSideStatementPgBeginExecutor implements ClientSideStatementExecutor } @Override - public StatementResult execute(ConnectionStatementExecutor connection, String sql) + public StatementResult execute(ConnectionStatementExecutor connection, ParsedStatement statement) throws Exception { - return (StatementResult) method.invoke(connection, getParameterValue(sql)); + return (StatementResult) + method.invoke(connection, getParameterValue(statement.getSqlWithoutComments())); } PgTransactionMode getParameterValue(String sql) { diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ClientSideStatementSetExecutor.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ClientSideStatementSetExecutor.java index 4f4fe140428..413905116ff 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ClientSideStatementSetExecutor.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ClientSideStatementSetExecutor.java @@ -18,6 +18,7 @@ import com.google.cloud.spanner.ErrorCode; import com.google.cloud.spanner.SpannerExceptionFactory; +import com.google.cloud.spanner.connection.AbstractStatementParser.ParsedStatement; import com.google.cloud.spanner.connection.ClientSideStatementImpl.CompileException; import com.google.common.base.Preconditions; import java.lang.reflect.Constructor; @@ -72,9 +73,10 @@ class ClientSideStatementSetExecutor implements ClientSideStatementExecutor { } @Override - public StatementResult execute(ConnectionStatementExecutor connection, String sql) + public StatementResult execute(ConnectionStatementExecutor connection, ParsedStatement statement) throws Exception { - return (StatementResult) method.invoke(connection, getParameterValue(sql)); + return (StatementResult) + method.invoke(connection, getParameterValue(statement.getSqlWithoutComments())); } T getParameterValue(String sql) { diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java index b6e29dbdc0e..989c2d30d1c 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java @@ -925,7 +925,7 @@ public StatementResult execute(Statement statement) { case CLIENT_SIDE: return parsedStatement .getClientSideStatement() - .execute(connectionStatementExecutor, parsedStatement.getSqlWithoutComments()); + .execute(connectionStatementExecutor, parsedStatement); case QUERY: return StatementResultImpl.of( internalExecuteQuery(CallType.SYNC, parsedStatement, AnalyzeMode.NONE)); @@ -957,7 +957,7 @@ public AsyncStatementResult executeAsync(Statement statement) { return AsyncStatementResultImpl.of( parsedStatement .getClientSideStatement() - .execute(connectionStatementExecutor, parsedStatement.getSqlWithoutComments()), + .execute(connectionStatementExecutor, parsedStatement), spanner.getAsyncExecutorProvider()); case QUERY: return AsyncStatementResultImpl.of( @@ -1010,7 +1010,7 @@ private ResultSet parseAndExecuteQuery( case CLIENT_SIDE: return parsedStatement .getClientSideStatement() - .execute(connectionStatementExecutor, parsedStatement.getSqlWithoutComments()) + .execute(connectionStatementExecutor, parsedStatement) .getResultSet(); case QUERY: return internalExecuteQuery(callType, parsedStatement, analyzeMode, options); @@ -1050,7 +1050,7 @@ private AsyncResultSet parseAndExecuteQueryAsync( return ResultSets.toAsyncResultSet( parsedStatement .getClientSideStatement() - .execute(connectionStatementExecutor, parsedStatement.getSqlWithoutComments()) + .execute(connectionStatementExecutor, parsedStatement) .getResultSet(), spanner.getAsyncExecutorProvider(), options); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/BeginPgTransactionTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/BeginPgTransactionTest.java index 9b2aa0930cf..2d2ef0781f0 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/BeginPgTransactionTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/BeginPgTransactionTest.java @@ -65,7 +65,7 @@ public void testBeginWithNoOption() { "start work isolation level serializable")) { ParsedStatement statement = parser.parse(Statement.of(sql)); assertEquals(sql, StatementType.CLIENT_SIDE, statement.getType()); - statement.getClientSideStatement().execute(executor, sql); + statement.getClientSideStatement().execute(executor, statement); verify(connection, times(index)).beginTransaction(); verify(connection, never()).setTransactionMode(any()); @@ -89,7 +89,7 @@ public void testBeginReadOnly() { "start work read only")) { ParsedStatement statement = parser.parse(Statement.of(sql)); assertEquals(sql, StatementType.CLIENT_SIDE, statement.getType()); - statement.getClientSideStatement().execute(executor, sql); + statement.getClientSideStatement().execute(executor, statement); verify(connection, times(index)).beginTransaction(); verify(connection, times(index)).setTransactionMode(TransactionMode.READ_ONLY_TRANSACTION); @@ -114,7 +114,7 @@ public void testBeginReadWrite() { "start work read write")) { ParsedStatement statement = parser.parse(Statement.of(sql)); assertEquals(sql, StatementType.CLIENT_SIDE, statement.getType()); - statement.getClientSideStatement().execute(executor, sql); + statement.getClientSideStatement().execute(executor, statement); verify(connection, times(index)).beginTransaction(); verify(connection, times(index)).setTransactionMode(TransactionMode.READ_WRITE_TRANSACTION); @@ -140,7 +140,7 @@ public void testBeginReadOnlyWithIsolationLevel() { "begin read write , \nisolation level default\n\t,read only")) { ParsedStatement statement = parser.parse(Statement.of(sql)); assertEquals(sql, StatementType.CLIENT_SIDE, statement.getType()); - statement.getClientSideStatement().execute(executor, sql); + statement.getClientSideStatement().execute(executor, statement); verify(connection, times(index)).beginTransaction(); verify(connection, times(index)).setTransactionMode(TransactionMode.READ_ONLY_TRANSACTION); @@ -173,7 +173,7 @@ public void testBeginWithNotDeferrable() { "begin not deferrable read write , \nisolation level default\n\t,read only")) { ParsedStatement statement = parser.parse(Statement.of(sql)); assertEquals(sql, StatementType.CLIENT_SIDE, statement.getType()); - statement.getClientSideStatement().execute(executor, sql); + statement.getClientSideStatement().execute(executor, statement); verify(connection, times(index)).beginTransaction(); verify(connection, times(index)).setTransactionMode(TransactionMode.READ_ONLY_TRANSACTION); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionStatementWithNoParametersTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionStatementWithNoParametersTest.java index 6140825f57b..0503c6c38d7 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionStatementWithNoParametersTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionStatementWithNoParametersTest.java @@ -51,6 +51,10 @@ public void setup() { parser = AbstractStatementParser.getInstance(dialect); } + ParsedStatement parse(String sql) { + return parser.parse(Statement.of(sql)); + } + @Test public void testExecuteGetAutocommit() { ParsedStatement statement = parser.parse(Statement.of("show variable autocommit")); @@ -58,7 +62,7 @@ public void testExecuteGetAutocommit() { ConnectionStatementExecutorImpl executor = mock(ConnectionStatementExecutorImpl.class); when(executor.getConnection()).thenReturn(connection); when(executor.statementShowAutocommit()).thenCallRealMethod(); - statement.getClientSideStatement().execute(executor, "show variable autocommit"); + statement.getClientSideStatement().execute(executor, parse("show variable autocommit")); verify(connection, times(1)).isAutocommit(); } @@ -72,7 +76,7 @@ public void testExecuteGetReadOnly() { ConnectionStatementExecutorImpl executor = new ConnectionStatementExecutorImpl(connection); statement .getClientSideStatement() - .execute(executor, String.format("show variable %sreadonly", getNamespace(dialect))); + .execute(executor, parse(String.format("show variable %sreadonly", getNamespace(dialect)))); verify(connection, times(1)).isReadOnly(); } @@ -89,7 +93,8 @@ public void testExecuteGetAutocommitDmlMode() { statement .getClientSideStatement() .execute( - executor, String.format("show variable %sautocommit_dml_mode", getNamespace(dialect))); + executor, + parse(String.format("show variable %sautocommit_dml_mode", getNamespace(dialect)))); verify(connection, times(1)).getAutocommitDmlMode(); } @@ -102,7 +107,7 @@ public void testExecuteGetStatementTimeout() { when(executor.statementShowStatementTimeout()).thenCallRealMethod(); when(connection.hasStatementTimeout()).thenReturn(true); when(connection.getStatementTimeout(TimeUnit.NANOSECONDS)).thenReturn(1L); - statement.getClientSideStatement().execute(executor, "show variable statement_timeout"); + statement.getClientSideStatement().execute(executor, parse("show variable statement_timeout")); verify(connection, times(2)).getStatementTimeout(TimeUnit.NANOSECONDS); } @@ -117,7 +122,9 @@ public void testExecuteGetReadTimestamp() { when(connection.getReadTimestampOrNull()).thenReturn(Timestamp.now()); statement .getClientSideStatement() - .execute(executor, String.format("show variable %sread_timestamp", getNamespace(dialect))); + .execute( + executor, + parse(String.format("show variable %sread_timestamp", getNamespace(dialect)))); verify(connection, times(1)).getReadTimestampOrNull(); } @@ -133,7 +140,8 @@ public void testExecuteGetCommitTimestamp() { statement .getClientSideStatement() .execute( - executor, String.format("show variable %scommit_timestamp", getNamespace(dialect))); + executor, + parse(String.format("show variable %scommit_timestamp", getNamespace(dialect)))); verify(connection, times(1)).getCommitTimestampOrNull(); } @@ -150,7 +158,8 @@ public void testExecuteGetReadOnlyStaleness() { statement .getClientSideStatement() .execute( - executor, String.format("show variable %sread_only_staleness", getNamespace(dialect))); + executor, + parse(String.format("show variable %sread_only_staleness", getNamespace(dialect)))); verify(connection, times(1)).getReadOnlyStaleness(); } @@ -167,7 +176,8 @@ public void testExecuteGetOptimizerVersion() { statement .getClientSideStatement() .execute( - executor, String.format("show variable %soptimizer_version", getNamespace(dialect))); + executor, + parse(String.format("show variable %soptimizer_version", getNamespace(dialect)))); verify(connection, times(1)).getOptimizerVersion(); } @@ -186,7 +196,9 @@ public void testExecuteGetOptimizerStatisticsPackage() { .getClientSideStatement() .execute( executor, - String.format("show variable %soptimizer_statistics_package", getNamespace(dialect))); + parse( + String.format( + "show variable %soptimizer_statistics_package", getNamespace(dialect)))); verify(connection, times(1)).getOptimizerStatisticsPackage(); } @@ -196,7 +208,7 @@ public void testExecuteBegin() { for (String statement : subject.getClientSideStatement().getExampleStatements()) { ConnectionImpl connection = mock(ConnectionImpl.class); ConnectionStatementExecutorImpl executor = new ConnectionStatementExecutorImpl(connection); - subject.getClientSideStatement().execute(executor, statement); + subject.getClientSideStatement().execute(executor, parse(statement)); verify(connection, times(1)).beginTransaction(); } } @@ -209,7 +221,7 @@ public void testExecuteCommit() { ConnectionStatementExecutorImpl executor = mock(ConnectionStatementExecutorImpl.class); when(executor.getConnection()).thenReturn(connection); when(executor.statementCommit()).thenCallRealMethod(); - subject.getClientSideStatement().execute(executor, statement); + subject.getClientSideStatement().execute(executor, parse(statement)); verify(connection, times(1)).commit(); } } @@ -222,7 +234,7 @@ public void testExecuteRollback() { ConnectionStatementExecutorImpl executor = mock(ConnectionStatementExecutorImpl.class); when(executor.getConnection()).thenReturn(connection); when(executor.statementRollback()).thenCallRealMethod(); - subject.getClientSideStatement().execute(executor, statement); + subject.getClientSideStatement().execute(executor, parse(statement)); verify(connection, times(1)).rollback(); } } diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionStatementWithOneParameterTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionStatementWithOneParameterTest.java index b68f78908b0..f4044316fc7 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionStatementWithOneParameterTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionStatementWithOneParameterTest.java @@ -55,6 +55,10 @@ public void setup() { parser = AbstractStatementParser.getInstance(dialect); } + ParsedStatement parse(String sql) { + return parser.parse(Statement.of(sql)); + } + @Test public void testExecuteSetAutocommit() { ParsedStatement subject = parser.parse(Statement.of("set autocommit = true")); @@ -65,7 +69,7 @@ public void testExecuteSetAutocommit() { for (Boolean mode : new Boolean[] {Boolean.FALSE, Boolean.TRUE}) { subject .getClientSideStatement() - .execute(executor, String.format("set autocommit = %s", mode)); + .execute(executor, parse(String.format("set autocommit = %s", mode))); verify(connection, times(1)).setAutocommit(mode); } } @@ -81,7 +85,8 @@ public void testExecuteSetReadOnly() { for (Boolean mode : new Boolean[] {Boolean.FALSE, Boolean.TRUE}) { subject .getClientSideStatement() - .execute(executor, String.format("set %sreadonly = %s", getNamespace(dialect), mode)); + .execute( + executor, parse(String.format("set %sreadonly = %s", getNamespace(dialect), mode))); verify(connection, times(1)).setReadOnly(mode); } } @@ -98,7 +103,8 @@ public void testExecuteSetReadOnlyTo() { for (Boolean mode : new Boolean[] {Boolean.FALSE, Boolean.TRUE}) { subject .getClientSideStatement() - .execute(executor, String.format("set %sreadonly to %s", getNamespace(dialect), mode)); + .execute( + executor, parse(String.format("set %sreadonly to %s", getNamespace(dialect), mode))); verify(connection, times(1)).setReadOnly(mode); } } @@ -117,7 +123,9 @@ public void testExecuteSetAutocommitDmlMode() { .getClientSideStatement() .execute( executor, - String.format("set %sautocommit_dml_mode='%s'", getNamespace(dialect), mode.name())); + parse( + String.format( + "set %sautocommit_dml_mode='%s'", getNamespace(dialect), mode.name()))); verify(connection, times(1)).setAutocommitDmlMode(mode); } } @@ -136,13 +144,7 @@ public void testExecuteSetStatementTimeout() { String.format( "set statement_timeout='%d%s'", val, ReadOnlyStalenessUtil.getTimeUnitAbbreviation(unit)))); - subject - .getClientSideStatement() - .execute( - executor, - String.format( - "set statement_timeout='%d%s'", - val, ReadOnlyStalenessUtil.getTimeUnitAbbreviation(unit))); + subject.getClientSideStatement().execute(executor, subject); verify(connection, times(1)).setStatementTimeout(val, unit); } } @@ -151,17 +153,15 @@ public void testExecuteSetStatementTimeout() { Mockito.clearInvocations(connection); ParsedStatement subject = parser.parse(Statement.of(String.format("set statement_timeout=%d", val))); - subject - .getClientSideStatement() - .execute(executor, String.format("set statement_timeout=%d", val)); + subject.getClientSideStatement().execute(executor, subject); verify(connection, times(1)).setStatementTimeout(val, TimeUnit.MILLISECONDS); } ParsedStatement subject = parser.parse(Statement.of("set statement_timeout=default")); - subject.getClientSideStatement().execute(executor, "set statement_timeout=default"); + subject.getClientSideStatement().execute(executor, subject); } else { ParsedStatement subject = parser.parse(Statement.of("set statement_timeout=null")); - subject.getClientSideStatement().execute(executor, "set statement_timeout=null"); + subject.getClientSideStatement().execute(executor, subject); } verify(connection, times(1)).clearStatementTimeout(); } @@ -187,9 +187,10 @@ public void testExecuteSetReadOnlyStaleness() { .getClientSideStatement() .execute( executor, - String.format( - "set %sread_only_staleness='%s'", - getNamespace(dialect), timestampBoundToString(val))); + parse( + String.format( + "set %sread_only_staleness='%s'", + getNamespace(dialect), timestampBoundToString(val)))); verify(connection, times(1)).setReadOnlyStaleness(val); } } @@ -225,7 +226,7 @@ public void testExecuteSetOptimizerVersion() { .getClientSideStatement() .execute( executor, - String.format("set %soptimizer_version='%s'", getNamespace(dialect), version)); + parse(String.format("set %soptimizer_version='%s'", getNamespace(dialect), version))); verify(connection, times(1)).setOptimizerVersion(version); } } @@ -245,9 +246,10 @@ public void testExecuteSetOptimizerStatisticsPackage() { .getClientSideStatement() .execute( executor, - String.format( - "set %soptimizer_statistics_package='%s'", - getNamespace(dialect), statisticsPackage)); + parse( + String.format( + "set %soptimizer_statistics_package='%s'", + getNamespace(dialect), statisticsPackage))); verify(connection, times(1)).setOptimizerStatisticsPackage(statisticsPackage); } } @@ -260,7 +262,7 @@ public void testExecuteSetTransaction() { for (TransactionMode mode : TransactionMode.values()) { subject .getClientSideStatement() - .execute(executor, String.format("set transaction %s", mode.getStatementString())); + .execute(executor, parse(String.format("set transaction %s", mode.getStatementString()))); verify(connection, times(1)).setTransactionMode(mode); } } diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/SetPgSessionCharacteristicsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/SetPgSessionCharacteristicsTest.java index 77240b74595..97394f67d0a 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/SetPgSessionCharacteristicsTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/SetPgSessionCharacteristicsTest.java @@ -44,7 +44,7 @@ public void testSetIsolationLevelDefault() { String sql = "set session characteristics as transaction isolation level default"; ParsedStatement statement = parser.parse(Statement.of(sql)); assertEquals(sql, StatementType.CLIENT_SIDE, statement.getType()); - statement.getClientSideStatement().execute(executor, sql); + statement.getClientSideStatement().execute(executor, statement); verify(connection, never()).setReadOnly(anyBoolean()); } @@ -57,7 +57,7 @@ public void testSetIsolationLevelSerializable() { String sql = "set session characteristics as transaction isolation level serializable"; ParsedStatement statement = parser.parse(Statement.of(sql)); assertEquals(sql, StatementType.CLIENT_SIDE, statement.getType()); - statement.getClientSideStatement().execute(executor, sql); + statement.getClientSideStatement().execute(executor, statement); verify(connection, never()).setReadOnly(anyBoolean()); } @@ -70,7 +70,7 @@ public void testSetIsolationLevelReadOnly() { String sql = "set\tsession\ncharacteristics as transaction read only"; ParsedStatement statement = parser.parse(Statement.of(sql)); assertEquals(sql, StatementType.CLIENT_SIDE, statement.getType()); - statement.getClientSideStatement().execute(executor, sql); + statement.getClientSideStatement().execute(executor, statement); verify(connection).setReadOnly(true); verify(connection, never()).setReadOnly(false); @@ -84,7 +84,7 @@ public void testSetIsolationLevelReadWrite() { String sql = "set session characteristics as transaction read write"; ParsedStatement statement = parser.parse(Statement.of(sql)); assertEquals(sql, StatementType.CLIENT_SIDE, statement.getType()); - statement.getClientSideStatement().execute(executor, sql); + statement.getClientSideStatement().execute(executor, statement); verify(connection).setReadOnly(false); verify(connection, never()).setReadOnly(true); @@ -99,7 +99,7 @@ public void testSetIsolationLevelSerializableReadWrite() { "set session characteristics as transaction isolation level serializable read write"; ParsedStatement statement = parser.parse(Statement.of(sql)); assertEquals(sql, StatementType.CLIENT_SIDE, statement.getType()); - statement.getClientSideStatement().execute(executor, sql); + statement.getClientSideStatement().execute(executor, statement); verify(connection).setReadOnly(false); verify(connection, never()).setReadOnly(true); @@ -114,7 +114,7 @@ public void testSetIsolationLevelSerializableReadOnly() { "set session characteristics as transaction isolation level serializable read only"; ParsedStatement statement = parser.parse(Statement.of(sql)); assertEquals(sql, StatementType.CLIENT_SIDE, statement.getType()); - statement.getClientSideStatement().execute(executor, sql); + statement.getClientSideStatement().execute(executor, statement); verify(connection).setReadOnly(true); } @@ -128,7 +128,7 @@ public void testSetMultipleTransactionModes() { "set session characteristics as transaction isolation level default, read only, isolation level serializable, read write"; ParsedStatement statement = parser.parse(Statement.of(sql)); assertEquals(sql, StatementType.CLIENT_SIDE, statement.getType()); - statement.getClientSideStatement().execute(executor, sql); + statement.getClientSideStatement().execute(executor, statement); verify(connection).setReadOnly(false); verify(connection, never()).setReadOnly(true); @@ -154,7 +154,7 @@ public void testDefaultTransactionIsolation() { }) { ParsedStatement statement = parser.parse(Statement.of(sql)); assertEquals(sql, StatementType.CLIENT_SIDE, statement.getType()); - statement.getClientSideStatement().execute(executor, sql); + statement.getClientSideStatement().execute(executor, statement); } // Setting the isolation level is a no-op. @@ -189,7 +189,7 @@ public void testDefaultTransactionReadOnlyTrue() { for (String sql : statements) { ParsedStatement statement = parser.parse(Statement.of(sql)); assertEquals(sql, StatementType.CLIENT_SIDE, statement.getType()); - statement.getClientSideStatement().execute(executor, sql); + statement.getClientSideStatement().execute(executor, statement); } verify(connection, times(statements.length)).setReadOnly(true); @@ -223,7 +223,7 @@ public void testDefaultTransactionReadOnlyFalse() { for (String sql : statements) { ParsedStatement statement = parser.parse(Statement.of(sql)); assertEquals(sql, StatementType.CLIENT_SIDE, statement.getType()); - statement.getClientSideStatement().execute(executor, sql); + statement.getClientSideStatement().execute(executor, statement); } verify(connection, times(statements.length)).setReadOnly(false); From f0e9f5b892425478b8983806c4b35183ad6bea13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Fri, 28 Jul 2023 16:22:06 +0200 Subject: [PATCH 2/2] chore: simplify test --- ...nnectionStatementWithNoParametersTest.java | 46 ++++--------------- 1 file changed, 9 insertions(+), 37 deletions(-) diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionStatementWithNoParametersTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionStatementWithNoParametersTest.java index 0503c6c38d7..4ea7bfc93de 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionStatementWithNoParametersTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionStatementWithNoParametersTest.java @@ -62,7 +62,7 @@ public void testExecuteGetAutocommit() { ConnectionStatementExecutorImpl executor = mock(ConnectionStatementExecutorImpl.class); when(executor.getConnection()).thenReturn(connection); when(executor.statementShowAutocommit()).thenCallRealMethod(); - statement.getClientSideStatement().execute(executor, parse("show variable autocommit")); + statement.getClientSideStatement().execute(executor, statement); verify(connection, times(1)).isAutocommit(); } @@ -74,9 +74,7 @@ public void testExecuteGetReadOnly() { ConnectionImpl connection = mock(ConnectionImpl.class); when(connection.getDialect()).thenReturn(dialect); ConnectionStatementExecutorImpl executor = new ConnectionStatementExecutorImpl(connection); - statement - .getClientSideStatement() - .execute(executor, parse(String.format("show variable %sreadonly", getNamespace(dialect)))); + statement.getClientSideStatement().execute(executor, statement); verify(connection, times(1)).isReadOnly(); } @@ -90,11 +88,7 @@ public void testExecuteGetAutocommitDmlMode() { when(connection.getDialect()).thenReturn(dialect); ConnectionStatementExecutorImpl executor = new ConnectionStatementExecutorImpl(connection); when(connection.getAutocommitDmlMode()).thenReturn(AutocommitDmlMode.TRANSACTIONAL); - statement - .getClientSideStatement() - .execute( - executor, - parse(String.format("show variable %sautocommit_dml_mode", getNamespace(dialect)))); + statement.getClientSideStatement().execute(executor, statement); verify(connection, times(1)).getAutocommitDmlMode(); } @@ -107,7 +101,7 @@ public void testExecuteGetStatementTimeout() { when(executor.statementShowStatementTimeout()).thenCallRealMethod(); when(connection.hasStatementTimeout()).thenReturn(true); when(connection.getStatementTimeout(TimeUnit.NANOSECONDS)).thenReturn(1L); - statement.getClientSideStatement().execute(executor, parse("show variable statement_timeout")); + statement.getClientSideStatement().execute(executor, statement); verify(connection, times(2)).getStatementTimeout(TimeUnit.NANOSECONDS); } @@ -120,11 +114,7 @@ public void testExecuteGetReadTimestamp() { when(connection.getDialect()).thenReturn(dialect); ConnectionStatementExecutorImpl executor = new ConnectionStatementExecutorImpl(connection); when(connection.getReadTimestampOrNull()).thenReturn(Timestamp.now()); - statement - .getClientSideStatement() - .execute( - executor, - parse(String.format("show variable %sread_timestamp", getNamespace(dialect)))); + statement.getClientSideStatement().execute(executor, statement); verify(connection, times(1)).getReadTimestampOrNull(); } @@ -137,11 +127,7 @@ public void testExecuteGetCommitTimestamp() { when(connection.getDialect()).thenReturn(dialect); ConnectionStatementExecutorImpl executor = new ConnectionStatementExecutorImpl(connection); when(connection.getCommitTimestampOrNull()).thenReturn(Timestamp.now()); - statement - .getClientSideStatement() - .execute( - executor, - parse(String.format("show variable %scommit_timestamp", getNamespace(dialect)))); + statement.getClientSideStatement().execute(executor, statement); verify(connection, times(1)).getCommitTimestampOrNull(); } @@ -155,11 +141,7 @@ public void testExecuteGetReadOnlyStaleness() { when(connection.getDialect()).thenReturn(dialect); ConnectionStatementExecutorImpl executor = new ConnectionStatementExecutorImpl(connection); when(connection.getReadOnlyStaleness()).thenReturn(TimestampBound.strong()); - statement - .getClientSideStatement() - .execute( - executor, - parse(String.format("show variable %sread_only_staleness", getNamespace(dialect)))); + statement.getClientSideStatement().execute(executor, statement); verify(connection, times(1)).getReadOnlyStaleness(); } @@ -173,11 +155,7 @@ public void testExecuteGetOptimizerVersion() { when(connection.getDialect()).thenReturn(dialect); ConnectionStatementExecutorImpl executor = new ConnectionStatementExecutorImpl(connection); when(connection.getOptimizerVersion()).thenReturn("1"); - statement - .getClientSideStatement() - .execute( - executor, - parse(String.format("show variable %soptimizer_version", getNamespace(dialect)))); + statement.getClientSideStatement().execute(executor, statement); verify(connection, times(1)).getOptimizerVersion(); } @@ -192,13 +170,7 @@ public void testExecuteGetOptimizerStatisticsPackage() { when(connection.getDialect()).thenReturn(dialect); ConnectionStatementExecutorImpl executor = new ConnectionStatementExecutorImpl(connection); when(connection.getOptimizerStatisticsPackage()).thenReturn("custom-package"); - statement - .getClientSideStatement() - .execute( - executor, - parse( - String.format( - "show variable %soptimizer_statistics_package", getNamespace(dialect)))); + statement.getClientSideStatement().execute(executor, statement); verify(connection, times(1)).getOptimizerStatisticsPackage(); }