Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat: support DML with Returning clause in Connection API #1978

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
55ae7db
feat: incorporate dml with returning clause
rajatbhatta Aug 17, 2022
f20d02f
feat: changes
rajatbhatta Aug 17, 2022
e06d436
feat: change handling of AsyncResultSet.
rajatbhatta Aug 17, 2022
bfbacfa
fix: lint
rajatbhatta Aug 17, 2022
3807e5f
doc: add comments
rajatbhatta Aug 17, 2022
595ee17
fix: lint
rajatbhatta Aug 17, 2022
f1326ff
test: add tests for executeBatchUpdate
rajatbhatta Aug 19, 2022
3f069bd
test: import fix
rajatbhatta Aug 19, 2022
ba692dc
test: remove redundant import
rajatbhatta Aug 19, 2022
ff3c0ff
test: add abort tests for dml returning
rajatbhatta Aug 19, 2022
5d1c0e5
test: add pg dml returning tests
rajatbhatta Aug 22, 2022
edaad92
feat: change error statement
rajatbhatta Aug 22, 2022
24cf255
doc: add doc for dml with returning clause usage
rajatbhatta Aug 22, 2022
3284f66
Merge branch 'googleapis:main' into dml-returning-parsing-changes
rajatbhatta Aug 22, 2022
4024412
Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/co…
rajatbhatta Aug 23, 2022
4ae2f8d
Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/co…
rajatbhatta Aug 23, 2022
c601a50
fix: incorporate review comments
rajatbhatta Aug 23, 2022
14e741f
test: add more test cases
rajatbhatta Aug 23, 2022
64422eb
test: add todo
rajatbhatta Aug 23, 2022
82e35fe
test: add separate abort tests for dml returning
rajatbhatta Aug 23, 2022
a7db0c1
fix: add try-with-resources block around ResultSet
rajatbhatta Aug 23, 2022
cc00396
feat: enhancement by adding a pre-check
rajatbhatta Aug 24, 2022
c093b9b
feat: changes
rajatbhatta Aug 24, 2022
4af9cbe
test: delete unnecessary test
rajatbhatta Aug 24, 2022
9f59715
test: add few more tests to PG parser
rajatbhatta Aug 24, 2022
218c3b7
feat: method doc update
rajatbhatta Aug 25, 2022
ea4152f
test: nit fixes
rajatbhatta Aug 26, 2022
e2a463e
feat: handle another corner case
rajatbhatta Aug 30, 2022
60fcb4c
test: add another test
rajatbhatta Aug 30, 2022
2c862c4
Merge branch 'main' into dml-returning-parsing-changes
rajatbhatta Nov 9, 2022
58d0278
clirr fixes
rajatbhatta Nov 9, 2022
81b7908
revert env for integration tests
rajatbhatta Nov 9, 2022
d0e173d
remove comments
rajatbhatta Nov 9, 2022
9e15159
skip returning tests in emulator
rajatbhatta Nov 9, 2022
ac3ea4f
fix: linting
rajatbhatta Nov 9, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions google-cloud-spanner/clirr-ignored-differences.xml
Expand Up @@ -202,4 +202,9 @@
<className>com/google/cloud/spanner/DatabaseClient</className>
<method>java.lang.String getDatabaseRole()</method>
</difference>
<difference>
<differenceType>7013</differenceType>
<className>com/google/cloud/spanner/connection/AbstractStatementParser</className>
<method>boolean checkReturningClauseInternal(java.lang.String)</method>
</difference>
</differences>
Expand Up @@ -140,6 +140,7 @@ public static class ParsedStatement {
private final ClientSideStatementImpl clientSideStatement;
private final Statement statement;
private final String sqlWithoutComments;
private final boolean returningClause;

private static ParsedStatement clientSideStatement(
ClientSideStatementImpl clientSideStatement,
Expand All @@ -155,11 +156,13 @@ private static ParsedStatement ddl(Statement statement, String sqlWithoutComment
private static ParsedStatement query(
Statement statement, String sqlWithoutComments, QueryOptions defaultQueryOptions) {
return new ParsedStatement(
StatementType.QUERY, statement, sqlWithoutComments, defaultQueryOptions);
StatementType.QUERY, statement, sqlWithoutComments, defaultQueryOptions, false);
}

private static ParsedStatement update(Statement statement, String sqlWithoutComments) {
return new ParsedStatement(StatementType.UPDATE, statement, sqlWithoutComments);
private static ParsedStatement update(
Statement statement, String sqlWithoutComments, boolean returningClause) {
return new ParsedStatement(
StatementType.UPDATE, statement, sqlWithoutComments, returningClause);
}

private static ParsedStatement unknown(Statement statement, String sqlWithoutComments) {
Expand All @@ -176,23 +179,34 @@ private ParsedStatement(
this.clientSideStatement = clientSideStatement;
this.statement = statement;
this.sqlWithoutComments = sqlWithoutComments;
this.returningClause = false;
}

private ParsedStatement(
StatementType type,
Statement statement,
String sqlWithoutComments,
boolean returningClause) {
this(type, statement, sqlWithoutComments, null, returningClause);
}

private ParsedStatement(StatementType type, Statement statement, String sqlWithoutComments) {
this(type, statement, sqlWithoutComments, null);
this(type, statement, sqlWithoutComments, null, false);
}

private ParsedStatement(
StatementType type,
Statement statement,
String sqlWithoutComments,
QueryOptions defaultQueryOptions) {
QueryOptions defaultQueryOptions,
boolean returningClause) {
Preconditions.checkNotNull(type);
Preconditions.checkNotNull(statement);
this.type = type;
this.clientSideStatement = null;
this.statement = mergeQueryOptions(statement, defaultQueryOptions);
this.sqlWithoutComments = sqlWithoutComments;
this.returningClause = returningClause;
}

@Override
Expand All @@ -219,6 +233,12 @@ public StatementType getType() {
return type;
}

/** Returns whether the statement has a returning clause or not. * */
@InternalApi
public boolean hasReturningClause() {
return this.returningClause;
}

/**
* Returns true if the statement is a query that will return a {@link
* com.google.cloud.spanner.ResultSet}.
Expand Down Expand Up @@ -355,7 +375,7 @@ ParsedStatement parse(Statement statement, QueryOptions defaultQueryOptions) {
} else if (isQuery(sql)) {
return ParsedStatement.query(statement, sql, defaultQueryOptions);
} else if (isUpdateStatement(sql)) {
return ParsedStatement.update(statement, sql);
return ParsedStatement.update(statement, sql, checkReturningClause(sql));
} else if (isDdlStatement(sql)) {
return ParsedStatement.ddl(statement, sql);
}
Expand Down Expand Up @@ -460,6 +480,10 @@ private boolean statementStartsWith(String sql, Iterable<String> checkStatements
static final char SLASH = '/';
static final char ASTERISK = '*';
static final char DOLLAR = '$';
static final char SPACE = ' ';
static final char CLOSE_PARENTHESIS = ')';
static final char COMMA = ',';
static final char UNDERSCORE = '_';

/**
* Removes comments from and trims the given sql statement using the dialect of this parser.
Expand Down Expand Up @@ -522,4 +546,19 @@ static int countOccurrencesOf(char c, String string) {
}
return res;
}

/**
* Checks if the given SQL string contains a Returning clause. This method is used only in case of
* a DML statement.
*
* @param sql The sql string without comments that has to be evaluated.
* @return A boolean indicating whether the sql string has a Returning clause or not.
*/
@InternalApi
protected abstract boolean checkReturningClauseInternal(String sql);

@InternalApi
public boolean checkReturningClause(String sql) {
return checkReturningClauseInternal(sql);
}
}
Expand Up @@ -856,8 +856,8 @@ default RpcPriority getRPCPriority() {
* state. The returned value depends on the type of statement:
*
* <ul>
* <li>Queries will return a {@link ResultSet}
* <li>DML statements will return an update count
* <li>Queries and DML statements with returning clause will return a {@link ResultSet}.
* <li>Simple DML statements will return an update count
* <li>DDL statements will return a {@link ResultType#NO_RESULT}
* <li>Connection and transaction statements (SET AUTOCOMMIT=TRUE|FALSE, SHOW AUTOCOMMIT, SET
* TRANSACTION READ ONLY, etc) will return either a {@link ResultSet} or {@link
Expand All @@ -874,9 +874,9 @@ default RpcPriority getRPCPriority() {
* state asynchronously. The returned value depends on the type of statement:
*
* <ul>
* <li>Queries will return an {@link AsyncResultSet}
* <li>DML statements will return an {@link ApiFuture} with an update count that is done when
* the DML statement has been applied successfully, or that throws an {@link
* <li>Queries and DML statements with returning clause will return an {@link AsyncResultSet}.
* <li>Simple DML statements will return an {@link ApiFuture} with an update count that is done
* when the DML statement has been applied successfully, or that throws an {@link
* ExecutionException} if the DML statement failed.
* <li>DDL statements will return an {@link ApiFuture} containing a {@link Void} that is done
* when the DDL statement has been applied successfully, or that throws an {@link
Expand All @@ -894,31 +894,33 @@ default RpcPriority getRPCPriority() {
AsyncStatementResult executeAsync(Statement statement);

/**
* Executes the given statement as a query and returns the result as a {@link ResultSet}. This
* method blocks and waits for a response from Spanner. If the statement does not contain a valid
* query, the method will throw a {@link SpannerException}.
* Executes the given statement (a query or a DML statement with returning clause) and returns the
* result as a {@link ResultSet}. This method blocks and waits for a response from Spanner. If the
* statement does not contain a valid query or a DML statement with returning clause, the method
* will throw a {@link SpannerException}.
*
* @param query The query statement to execute
* @param query The query statement or DML statement with returning clause to execute
* @param options the options to configure the query
* @return a {@link ResultSet} with the results of the query
* @return a {@link ResultSet} with the results of the statement
*/
ResultSet executeQuery(Statement query, QueryOption... options);

/**
* Executes the given statement asynchronously as a query and returns the result as an {@link
* AsyncResultSet}. This method is guaranteed to be non-blocking. If the statement does not
* contain a valid query, the method will throw a {@link SpannerException}.
* Executes the given statement (a query or a DML statement with returning clause) asynchronously
* and returns the result as an {@link AsyncResultSet}. This method is guaranteed to be
* non-blocking. If the statement does not contain a valid query or a DML statement with returning
* clause, the method will throw a {@link SpannerException}.
*
* <p>See {@link AsyncResultSet#setCallback(java.util.concurrent.Executor,
* com.google.cloud.spanner.AsyncResultSet.ReadyCallback)} for more information on how to consume
* the results of the query asynchronously.
* the results of the statement asynchronously.
*
* <p>It is also possible to consume the returned {@link AsyncResultSet} in the same way as a
* normal {@link ResultSet}, i.e. in a while-loop calling {@link AsyncResultSet#next()}.
*
* @param query The query statement to execute
* @param query The query statement or DML statement with returning clause to execute
* @param options the options to configure the query
* @return an {@link AsyncResultSet} with the results of the query
* @return an {@link AsyncResultSet} with the results of the statement
*/
AsyncResultSet executeQueryAsync(Statement query, QueryOption... options);

Expand Down Expand Up @@ -951,8 +953,8 @@ default RpcPriority getRPCPriority() {
ResultSet analyzeQuery(Statement query, QueryAnalyzeMode queryMode);

/**
* Executes the given statement as a DML statement. If the statement does not contain a valid DML
* statement, the method will throw a {@link SpannerException}.
* Executes the given statement as a simple DML statement. If the statement does not contain a
* valid DML statement, the method will throw a {@link SpannerException}.
*
* @param update The update statement to execute
* @return the number of records that were inserted/updated/deleted by this statement
Expand All @@ -972,8 +974,9 @@ default ResultSetStats analyzeUpdate(Statement update, QueryAnalyzeMode analyzeM
}

/**
* Executes the given statement asynchronously as a DML statement. If the statement does not
* contain a valid DML statement, the method will throw a {@link SpannerException}.
* Executes the given statement asynchronously as a simple DML statement. If the statement does
* not contain a simple DML statement, the method will throw a {@link SpannerException}. A DML
* statement with returning clause will throw a {@link SpannerException}.
*
* <p>This method is guaranteed to be non-blocking.
*
Expand All @@ -984,8 +987,9 @@ default ResultSetStats analyzeUpdate(Statement update, QueryAnalyzeMode analyzeM
ApiFuture<Long> executeUpdateAsync(Statement update);

/**
* Executes a list of DML statements in a single request. The statements will be executed in order
* and the semantics is the same as if each statement is executed by {@link
* Executes a list of DML statements (can be simple DML statements or DML statements with
* returning clause) in a single request. The statements will be executed in order and the
rajatbhatta marked this conversation as resolved.
Show resolved Hide resolved
* semantics is the same as if each statement is executed by {@link
* Connection#executeUpdate(Statement)} in a loop. This method returns an array of long integers,
* each representing the number of rows modified by each statement.
*
Expand All @@ -1006,8 +1010,9 @@ default ResultSetStats analyzeUpdate(Statement update, QueryAnalyzeMode analyzeM
long[] executeBatchUpdate(Iterable<Statement> updates);

/**
* Executes a list of DML statements in a single request. The statements will be executed in order
* and the semantics is the same as if each statement is executed by {@link
* Executes a list of DML statements (can be simple DML statements or DML statements with
* returning clause) in a single request. The statements will be executed in order and the
rajatbhatta marked this conversation as resolved.
Show resolved Hide resolved
* semantics is the same as if each statement is executed by {@link
* Connection#executeUpdate(Statement)} in a loop. This method returns an {@link ApiFuture} that
* contains an array of long integers, each representing the number of rows modified by each
* statement.
Expand Down
Expand Up @@ -853,6 +853,9 @@ public StatementResult execute(Statement statement) {
case QUERY:
return StatementResultImpl.of(internalExecuteQuery(parsedStatement, AnalyzeMode.NONE));
case UPDATE:
if (parsedStatement.hasReturningClause()) {
return StatementResultImpl.of(internalExecuteQuery(parsedStatement, AnalyzeMode.NONE));
}
return StatementResultImpl.of(get(internalExecuteUpdateAsync(parsedStatement)));
case DDL:
get(executeDdlAsync(parsedStatement));
Expand Down Expand Up @@ -881,6 +884,10 @@ public AsyncStatementResult executeAsync(Statement statement) {
return AsyncStatementResultImpl.of(
internalExecuteQueryAsync(parsedStatement, AnalyzeMode.NONE));
case UPDATE:
if (parsedStatement.hasReturningClause()) {
return AsyncStatementResultImpl.of(
internalExecuteQueryAsync(parsedStatement, AnalyzeMode.NONE));
}
return AsyncStatementResultImpl.of(internalExecuteUpdateAsync(parsedStatement));
case DDL:
return AsyncStatementResultImpl.noResult(executeDdlAsync(parsedStatement));
Expand Down Expand Up @@ -918,7 +925,7 @@ private ResultSet parseAndExecuteQuery(
Preconditions.checkNotNull(analyzeMode);
ConnectionPreconditions.checkState(!isClosed(), CLOSED_ERROR_MSG);
ParsedStatement parsedStatement = getStatementParser().parse(query, this.queryOptions);
if (parsedStatement.isQuery()) {
if (parsedStatement.isQuery() || parsedStatement.isUpdate()) {
switch (parsedStatement.getType()) {
case CLIENT_SIDE:
return parsedStatement
Expand All @@ -928,22 +935,36 @@ private ResultSet parseAndExecuteQuery(
case QUERY:
return internalExecuteQuery(parsedStatement, analyzeMode, options);
case UPDATE:
if (parsedStatement.hasReturningClause()) {
// Cannot execute DML statement with returning clause in read-only mode or in
// READ_ONLY_TRANSACTION transaction mode.
if (this.isReadOnly()
|| (this.isInTransaction()
&& this.getTransactionMode() == TransactionMode.READ_ONLY_TRANSACTION)) {
throw SpannerExceptionFactory.newSpannerException(
ErrorCode.FAILED_PRECONDITION,
"DML statement with returning clause cannot be executed in read-only mode: "
+ parsedStatement.getSqlWithoutComments());
}
return internalExecuteQuery(parsedStatement, analyzeMode, options);
}
case DDL:
case UNKNOWN:
default:
}
}
throw SpannerExceptionFactory.newSpannerException(
ErrorCode.INVALID_ARGUMENT,
"Statement is not a query: " + parsedStatement.getSqlWithoutComments());
"Statement is not a query or DML with returning clause: "
+ parsedStatement.getSqlWithoutComments());
}

private AsyncResultSet parseAndExecuteQueryAsync(
Statement query, AnalyzeMode analyzeMode, QueryOption... options) {
Preconditions.checkNotNull(query);
ConnectionPreconditions.checkState(!isClosed(), CLOSED_ERROR_MSG);
ParsedStatement parsedStatement = getStatementParser().parse(query, this.queryOptions);
if (parsedStatement.isQuery()) {
if (parsedStatement.isQuery() || parsedStatement.isUpdate()) {
switch (parsedStatement.getType()) {
case CLIENT_SIDE:
return ResultSets.toAsyncResultSet(
Expand All @@ -956,14 +977,28 @@ private AsyncResultSet parseAndExecuteQueryAsync(
case QUERY:
return internalExecuteQueryAsync(parsedStatement, analyzeMode, options);
case UPDATE:
if (parsedStatement.hasReturningClause()) {
// Cannot execute DML statement with returning clause in read-only mode or in
// READ_ONLY_TRANSACTION transaction mode.
if (this.isReadOnly()
|| (this.isInTransaction()
&& this.getTransactionMode() == TransactionMode.READ_ONLY_TRANSACTION)) {
throw SpannerExceptionFactory.newSpannerException(
ErrorCode.FAILED_PRECONDITION,
"DML statement with returning clause cannot be executed in read-only mode: "
+ parsedStatement.getSqlWithoutComments());
}
return internalExecuteQueryAsync(parsedStatement, analyzeMode, options);
}
case DDL:
case UNKNOWN:
default:
}
}
throw SpannerExceptionFactory.newSpannerException(
ErrorCode.INVALID_ARGUMENT,
"Statement is not a query: " + parsedStatement.getSqlWithoutComments());
"Statement is not a query or DML with returning clause: "
+ parsedStatement.getSqlWithoutComments());
}

@Override
Expand All @@ -974,6 +1009,13 @@ public long executeUpdate(Statement update) {
if (parsedStatement.isUpdate()) {
switch (parsedStatement.getType()) {
case UPDATE:
if (parsedStatement.hasReturningClause()) {
throw SpannerExceptionFactory.newSpannerException(
ErrorCode.FAILED_PRECONDITION,
"DML statement with returning clause cannot be executed using executeUpdate: "
+ parsedStatement.getSqlWithoutComments()
+ ". Please use executeQuery instead.");
}
return get(internalExecuteUpdateAsync(parsedStatement));
case CLIENT_SIDE:
case QUERY:
Expand All @@ -995,6 +1037,13 @@ public ApiFuture<Long> executeUpdateAsync(Statement update) {
if (parsedStatement.isUpdate()) {
switch (parsedStatement.getType()) {
case UPDATE:
if (parsedStatement.hasReturningClause()) {
throw SpannerExceptionFactory.newSpannerException(
ErrorCode.FAILED_PRECONDITION,
"DML statement with returning clause cannot be executed using executeUpdateAsync: "
+ parsedStatement.getSqlWithoutComments()
+ ". Please use executeQueryAsync instead.");
}
return internalExecuteUpdateAsync(parsedStatement);
case CLIENT_SIDE:
case QUERY:
Expand Down Expand Up @@ -1141,8 +1190,9 @@ private ResultSet internalExecuteQuery(
final QueryOption... options) {
Preconditions.checkArgument(
statement.getType() == StatementType.QUERY
|| (statement.getType() == StatementType.UPDATE && analyzeMode != AnalyzeMode.NONE),
"Statement must either be a query or a DML mode with analyzeMode!=NONE");
|| (statement.getType() == StatementType.UPDATE
&& (analyzeMode != AnalyzeMode.NONE || statement.hasReturningClause())),
"Statement must either be a query or a DML mode with analyzeMode!=NONE or returning clause");
UnitOfWork transaction = getCurrentUnitOfWorkOrStartNewUnitOfWork();
return get(
transaction.executeQueryAsync(
Expand All @@ -1154,7 +1204,9 @@ private AsyncResultSet internalExecuteQueryAsync(
final AnalyzeMode analyzeMode,
final QueryOption... options) {
Preconditions.checkArgument(
statement.getType() == StatementType.QUERY, "Statement must be a query");
(statement.getType() == StatementType.QUERY)
|| (statement.getType() == StatementType.UPDATE && statement.hasReturningClause()),
"Statement must be a query or DML with returning clause.");
UnitOfWork transaction = getCurrentUnitOfWorkOrStartNewUnitOfWork();
return ResultSets.toAsyncResultSet(
transaction.executeQueryAsync(
Expand Down