Skip to content

Commit 0e1e14c

Browse files
olavloiteolavloite
authored
perf: remove all calls to getSqlWithoutComments (#3822)
Calls to getSqlWithoutComments() have been replaced with calls to getSql(). This reduces the number of times that the SQL string needs to be parsed, and the number of strings that are created by the Connection API. The only part of the Connection API that still depends on the SQL string without comments, is the check whether a DML statement contains a THEN RETURN / RETURNING clause. This will be removed in a follow-up change.
1 parent c102cb4 commit 0e1e14c

28 files changed

+208
-93
lines changed

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/AbstractBaseUnitOfWork.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,7 @@ <ResponseT, MetadataT> ResponseT getWithStatementTimeout(
317317
} catch (TimeoutException e) {
318318
throw SpannerExceptionFactory.newSpannerException(
319319
ErrorCode.DEADLINE_EXCEEDED,
320-
"Statement execution timeout occurred for " + statement.getSqlWithoutComments(),
320+
"Statement execution timeout occurred for " + statement.getSql(),
321321
e);
322322
} catch (ExecutionException e) {
323323
Throwable cause = e.getCause();
@@ -331,7 +331,7 @@ <ResponseT, MetadataT> ResponseT getWithStatementTimeout(
331331
}
332332
throw SpannerExceptionFactory.newSpannerException(
333333
ErrorCode.fromGrpcStatus(Status.fromThrowable(e)),
334-
"Statement execution failed for " + statement.getSqlWithoutComments(),
334+
"Statement execution failed for " + statement.getSql(),
335335
e);
336336
} catch (InterruptedException e) {
337337
throw SpannerExceptionFactory.newSpannerException(

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/AbstractStatementParser.java‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -425,7 +425,17 @@ Statement mergeQueryOptions(Statement statement, QueryOptions defaultQueryOption
425425
.build();
426426
}
427427

428-
/** @return the SQL statement with all comments removed from the SQL string. */
428+
/** @return the original SQL statement */
429+
@InternalApi
430+
public String getSql() {
431+
return statement.getSql();
432+
}
433+
434+
/**
435+
* @return the SQL statement with all comments removed from the SQL string.
436+
* @deprecated use {@link #getSql()} instead
437+
*/
438+
@Deprecated
429439
@InternalApi
430440
public String getSqlWithoutComments() {
431441
return sqlWithoutComments.get();

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ClientSideStatementBeginExecutor.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,7 @@ class ClientSideStatementBeginExecutor implements ClientSideStatementExecutor {
4646
@Override
4747
public StatementResult execute(ConnectionStatementExecutor connection, ParsedStatement statement)
4848
throws Exception {
49-
return (StatementResult)
50-
method.invoke(connection, getParameterValue(statement.getSqlWithoutComments()));
49+
return (StatementResult) method.invoke(connection, getParameterValue(statement.getSql()));
5150
}
5251

5352
IsolationLevel getParameterValue(String sql) {

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ClientSideStatementExplainExecutor.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,7 @@ class ClientSideStatementExplainExecutor implements ClientSideStatementExecutor
5050
@Override
5151
public StatementResult execute(ConnectionStatementExecutor connection, ParsedStatement statement)
5252
throws Exception {
53-
return (StatementResult)
54-
method.invoke(connection, getParameterValue(statement.getSqlWithoutComments()));
53+
return (StatementResult) method.invoke(connection, getParameterValue(statement.getSql()));
5554
}
5655

5756
String getParameterValue(String sql) {

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ClientSideStatementPartitionExecutor.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public StatementResult execute(
4949
}
5050

5151
String getParameterValue(ParsedStatement parsedStatement) {
52-
Matcher matcher = statement.getPattern().matcher(parsedStatement.getSqlWithoutComments());
52+
Matcher matcher = statement.getPattern().matcher(parsedStatement.getSql());
5353
if (matcher.find() && matcher.groupCount() >= 2) {
5454
String space = matcher.group(1);
5555
String value = matcher.group(2);

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ClientSideStatementPgBeginExecutor.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,7 @@ class ClientSideStatementPgBeginExecutor implements ClientSideStatementExecutor
4545
@Override
4646
public StatementResult execute(ConnectionStatementExecutor connection, ParsedStatement statement)
4747
throws Exception {
48-
return (StatementResult)
49-
method.invoke(connection, getParameterValue(statement.getSqlWithoutComments()));
48+
return (StatementResult) method.invoke(connection, getParameterValue(statement.getSql()));
5049
}
5150

5251
PgTransactionMode getParameterValue(String sql) {

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ClientSideStatementRunPartitionExecutor.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ String getParameterValue(ParsedStatement parsedStatement) {
6565
// 2. If the matcher matches and returns zero groups, we know that the statement is valid, but
6666
// that it does not contain a partition-id in the SQL statement. The partition-id must then
6767
// be included in the statement as a query parameter.
68-
Matcher matcher = statement.getPattern().matcher(parsedStatement.getSqlWithoutComments());
68+
Matcher matcher = statement.getPattern().matcher(parsedStatement.getSql());
6969
if (matcher.find() && matcher.groupCount() >= 1) {
7070
String value = matcher.group(1);
7171
if (!Strings.isNullOrEmpty(value)) {

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ClientSideStatementRunPartitionedQueryExecutor.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public StatementResult execute(
5050
}
5151

5252
String getParameterValue(ParsedStatement parsedStatement) {
53-
Matcher matcher = statement.getPattern().matcher(parsedStatement.getSqlWithoutComments());
53+
Matcher matcher = statement.getPattern().matcher(parsedStatement.getSql());
5454
if (matcher.find() && matcher.groupCount() >= 2) {
5555
// Include the spacing group in case the query is enclosed in parentheses like this:
5656
// `run partitioned query(select * from foo)`

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ClientSideStatementSetExecutor.java‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package com.google.cloud.spanner.connection;
1818

1919
import com.google.cloud.Tuple;
20+
import com.google.cloud.spanner.Dialect;
2021
import com.google.cloud.spanner.ErrorCode;
2122
import com.google.cloud.spanner.SpannerExceptionFactory;
2223
import com.google.cloud.spanner.connection.AbstractStatementParser.ParsedStatement;
@@ -27,6 +28,7 @@
2728
import com.google.common.util.concurrent.UncheckedExecutionException;
2829
import java.lang.reflect.Constructor;
2930
import java.lang.reflect.Method;
31+
import java.nio.CharBuffer;
3032
import java.util.concurrent.ExecutionException;
3133
import java.util.regex.Matcher;
3234
import java.util.regex.Pattern;
@@ -104,8 +106,8 @@ public StatementResult execute(ConnectionStatementExecutor connection, ParsedSta
104106
try {
105107
value =
106108
this.cache.get(
107-
statement.getSqlWithoutComments(),
108-
() -> getParameterValue(statement.getSqlWithoutComments()));
109+
statement.getSql(),
110+
() -> getParameterValue(connection.getDialect(), statement.getSql()));
109111
} catch (ExecutionException | UncheckedExecutionException executionException) {
110112
throw SpannerExceptionFactory.asSpannerException(executionException.getCause());
111113
}
@@ -115,8 +117,13 @@ public StatementResult execute(ConnectionStatementExecutor connection, ParsedSta
115117
return (StatementResult) method.invoke(connection, value.x());
116118
}
117119

118-
Tuple<T, Boolean> getParameterValue(String sql) {
119-
Matcher matcher = allowedValuesPattern.matcher(sql);
120+
Tuple<T, Boolean> getParameterValue(Dialect dialect, String sql) {
121+
// Get rid of any spaces/comments at the start of the string.
122+
SimpleParser simpleParser = new SimpleParser(dialect, sql);
123+
simpleParser.skipWhitespaces();
124+
// Create a wrapper around the SQL string from the point after the first whitespace.
125+
CharBuffer sqlAfterWhitespaces = CharBuffer.wrap(sql, simpleParser.getPos(), sql.length());
126+
Matcher matcher = allowedValuesPattern.matcher(sqlAfterWhitespaces);
120127
if (matcher.find() && matcher.groupCount() >= 2) {
121128
boolean local = matcher.group(1) != null && "local".equalsIgnoreCase(matcher.group(1).trim());
122129
String value = matcher.group(2);
@@ -130,7 +137,7 @@ Tuple<T, Boolean> getParameterValue(String sql) {
130137
"Unknown value for %s: %s",
131138
this.statement.getSetStatement().getPropertyName(), value));
132139
} else {
133-
Matcher invalidMatcher = this.statement.getPattern().matcher(sql);
140+
Matcher invalidMatcher = this.statement.getPattern().matcher(sqlAfterWhitespaces);
134141
int valueGroup = this.supportsLocal ? 2 : 1;
135142
if (invalidMatcher.find() && invalidMatcher.groupCount() == valueGroup) {
136143
String invalidValue = invalidMatcher.group(valueGroup);

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java‎

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -382,6 +382,7 @@ public Spanner getSpanner() {
382382
private DdlClient createDdlClient() {
383383
return DdlClient.newBuilder()
384384
.setDatabaseAdminClient(spanner.getDatabaseAdminClient())
385+
.setDialectSupplier(this::getDialect)
385386
.setProjectId(options.getProjectId())
386387
.setInstanceId(options.getInstanceId())
387388
.setDatabaseName(options.getDatabaseName())
@@ -1424,8 +1425,7 @@ private StatementResult internalExecute(
14241425
default:
14251426
}
14261427
throw SpannerExceptionFactory.newSpannerException(
1427-
ErrorCode.INVALID_ARGUMENT,
1428-
"Unknown statement: " + parsedStatement.getSqlWithoutComments());
1428+
ErrorCode.INVALID_ARGUMENT, "Unknown statement: " + parsedStatement.getSql());
14291429
}
14301430

14311431
@VisibleForTesting
@@ -1470,8 +1470,7 @@ private static ResultType getResultType(ParsedStatement parsedStatement) {
14701470
case UNKNOWN:
14711471
default:
14721472
throw SpannerExceptionFactory.newSpannerException(
1473-
ErrorCode.INVALID_ARGUMENT,
1474-
"Unknown statement: " + parsedStatement.getSqlWithoutComments());
1473+
ErrorCode.INVALID_ARGUMENT, "Unknown statement: " + parsedStatement.getSql());
14751474
}
14761475
}
14771476

@@ -1503,8 +1502,7 @@ public AsyncStatementResult executeAsync(Statement statement) {
15031502
default:
15041503
}
15051504
throw SpannerExceptionFactory.newSpannerException(
1506-
ErrorCode.INVALID_ARGUMENT,
1507-
"Unknown statement: " + parsedStatement.getSqlWithoutComments());
1505+
ErrorCode.INVALID_ARGUMENT, "Unknown statement: " + parsedStatement.getSql());
15081506
}
15091507

15101508
@Override
@@ -1699,7 +1697,7 @@ private ResultSet parseAndExecuteQuery(
16991697
throw SpannerExceptionFactory.newSpannerException(
17001698
ErrorCode.FAILED_PRECONDITION,
17011699
"DML statement with returning clause cannot be executed in read-only mode: "
1702-
+ parsedStatement.getSqlWithoutComments());
1700+
+ parsedStatement.getSql());
17031701
}
17041702
return internalExecuteQuery(callType, parsedStatement, analyzeMode, options);
17051703
}
@@ -1710,8 +1708,7 @@ private ResultSet parseAndExecuteQuery(
17101708
}
17111709
throw SpannerExceptionFactory.newSpannerException(
17121710
ErrorCode.INVALID_ARGUMENT,
1713-
"Statement is not a query or DML with returning clause: "
1714-
+ parsedStatement.getSqlWithoutComments());
1711+
"Statement is not a query or DML with returning clause: " + parsedStatement.getSql());
17151712
}
17161713

17171714
private AsyncResultSet parseAndExecuteQueryAsync(Statement query, QueryOption... options) {
@@ -1741,7 +1738,7 @@ private AsyncResultSet parseAndExecuteQueryAsync(Statement query, QueryOption...
17411738
throw SpannerExceptionFactory.newSpannerException(
17421739
ErrorCode.FAILED_PRECONDITION,
17431740
"DML statement with returning clause cannot be executed in read-only mode: "
1744-
+ parsedStatement.getSqlWithoutComments());
1741+
+ parsedStatement.getSql());
17451742
}
17461743
return internalExecuteQueryAsync(
17471744
CallType.ASYNC, parsedStatement, AnalyzeMode.NONE, options);
@@ -1753,8 +1750,7 @@ private AsyncResultSet parseAndExecuteQueryAsync(Statement query, QueryOption...
17531750
}
17541751
throw SpannerExceptionFactory.newSpannerException(
17551752
ErrorCode.INVALID_ARGUMENT,
1756-
"Statement is not a query or DML with returning clause: "
1757-
+ parsedStatement.getSqlWithoutComments());
1753+
"Statement is not a query or DML with returning clause: " + parsedStatement.getSql());
17581754
}
17591755

17601756
private boolean isInternalMetadataQuery(QueryOption... options) {
@@ -1781,7 +1777,7 @@ public long executeUpdate(Statement update) {
17811777
throw SpannerExceptionFactory.newSpannerException(
17821778
ErrorCode.FAILED_PRECONDITION,
17831779
"DML statement with returning clause cannot be executed using executeUpdate: "
1784-
+ parsedStatement.getSqlWithoutComments()
1780+
+ parsedStatement.getSql()
17851781
+ ". Please use executeQuery instead.");
17861782
}
17871783
return get(internalExecuteUpdateAsync(CallType.SYNC, parsedStatement));
@@ -1794,7 +1790,7 @@ public long executeUpdate(Statement update) {
17941790
}
17951791
throw SpannerExceptionFactory.newSpannerException(
17961792
ErrorCode.INVALID_ARGUMENT,
1797-
"Statement is not an update statement: " + parsedStatement.getSqlWithoutComments());
1793+
"Statement is not an update statement: " + parsedStatement.getSql());
17981794
}
17991795

18001796
@Override
@@ -1809,7 +1805,7 @@ public ApiFuture<Long> executeUpdateAsync(Statement update) {
18091805
throw SpannerExceptionFactory.newSpannerException(
18101806
ErrorCode.FAILED_PRECONDITION,
18111807
"DML statement with returning clause cannot be executed using executeUpdateAsync: "
1812-
+ parsedStatement.getSqlWithoutComments()
1808+
+ parsedStatement.getSql()
18131809
+ ". Please use executeQueryAsync instead.");
18141810
}
18151811
return internalExecuteUpdateAsync(CallType.ASYNC, parsedStatement);
@@ -1822,7 +1818,7 @@ public ApiFuture<Long> executeUpdateAsync(Statement update) {
18221818
}
18231819
throw SpannerExceptionFactory.newSpannerException(
18241820
ErrorCode.INVALID_ARGUMENT,
1825-
"Statement is not an update statement: " + parsedStatement.getSqlWithoutComments());
1821+
"Statement is not an update statement: " + parsedStatement.getSql());
18261822
}
18271823

18281824
@Override
@@ -1845,7 +1841,7 @@ public ResultSetStats analyzeUpdate(Statement update, QueryAnalyzeMode analyzeMo
18451841
}
18461842
throw SpannerExceptionFactory.newSpannerException(
18471843
ErrorCode.INVALID_ARGUMENT,
1848-
"Statement is not an update statement: " + parsedStatement.getSqlWithoutComments());
1844+
"Statement is not an update statement: " + parsedStatement.getSql());
18491845
}
18501846

18511847
@Override
@@ -1867,7 +1863,7 @@ public ResultSet analyzeUpdateStatement(
18671863
}
18681864
throw SpannerExceptionFactory.newSpannerException(
18691865
ErrorCode.INVALID_ARGUMENT,
1870-
"Statement is not an update statement: " + parsedStatement.getSqlWithoutComments());
1866+
"Statement is not an update statement: " + parsedStatement.getSql());
18711867
}
18721868

18731869
@Override
@@ -1899,7 +1895,7 @@ private List<ParsedStatement> parseUpdateStatements(Iterable<Statement> updates)
18991895
throw SpannerExceptionFactory.newSpannerException(
19001896
ErrorCode.INVALID_ARGUMENT,
19011897
"The batch update list contains a statement that is not an update statement: "
1902-
+ parsedStatement.getSqlWithoutComments());
1898+
+ parsedStatement.getSql());
19031899
}
19041900
}
19051901
return parsedStatements;

0 commit comments

Comments
 (0)