Skip to content

Commit 22833ac

Browse files
olavloiteolavloite
authored
feat: allow DDL with autocommit=false (#3057)
Adds support for running DDL statements when a connection is in autocommit=false mode. By default, DDL statements are only allowed when no transaction is active. That is; no query or DML statement has been executed which activated a read/write transaction. A new flag is added that can be used to revert the behavior back to the original behavior where DDL is always refused when autocommit=false. The same flag can also be used to make the API behave the same as MySQL and Oracle, where any active transaction is automatically committed whenever a DDL statement is encountered. Concretely this means that the following is now allowed: ``` set autocommit=false; create table Singers (SingerId INT64, Name STRING(MAX)) PRIMARY KEY (SingerId); ``` The following is by default NOT allowed, unless ddlInTransactionMode=AUTO_COMMIT_TRANSACTION ``` set autocommit=false; select * from singers; -- This starts a transaction create table Albums (AlbumId INT64) PRIMARY KEY (AlbumId); -- This is not allowed ```
1 parent e595157 commit 22833ac

13 files changed

+3553
-256
lines changed

‎google-cloud-spanner/clirr-ignored-differences.xml

+12
Original file line numberDiff line numberDiff line change
@@ -656,5 +656,17 @@
656656
<className>com/google/cloud/spanner/connection/Connection</className>
657657
<method>com.google.cloud.spanner.Spanner getSpanner()</method>
658658
</difference>
659+
660+
<!-- Add DdlInTransactionMode -->
661+
<difference>
662+
<differenceType>7012</differenceType>
663+
<className>com/google/cloud/spanner/connection/Connection</className>
664+
<method>void setDdlInTransactionMode(com.google.cloud.spanner.connection.DdlInTransactionMode)</method>
665+
</difference>
666+
<difference>
667+
<differenceType>7012</differenceType>
668+
<className>com/google/cloud/spanner/connection/Connection</className>
669+
<method>com.google.cloud.spanner.connection.DdlInTransactionMode getDdlInTransactionMode()</method>
670+
</difference>
659671

660672
</differences>

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

+6
Original file line numberDiff line numberDiff line change
@@ -773,6 +773,12 @@ default boolean isDelayTransactionStartUntilFirstWrite() {
773773
/** Sets how savepoints should be supported on this connection. */
774774
void setSavepointSupport(SavepointSupport savepointSupport);
775775

776+
/** Returns the current {@link DdlInTransactionMode} for this connection. */
777+
DdlInTransactionMode getDdlInTransactionMode();
778+
779+
/** Sets how the connection should behave if a DDL statement is executed during a transaction. */
780+
void setDdlInTransactionMode(DdlInTransactionMode ddlInTransactionMode);
781+
776782
/**
777783
* Creates a savepoint with the given name.
778784
*

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

+64-10
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,7 @@ static UnitOfWorkType of(TransactionMode transactionMode) {
243243
private QueryOptions queryOptions = QueryOptions.getDefaultInstance();
244244
private RpcPriority rpcPriority = null;
245245
private SavepointSupport savepointSupport = SavepointSupport.FAIL_AFTER_ROLLBACK;
246+
private DdlInTransactionMode ddlInTransactionMode;
246247

247248
private String transactionTag;
248249
private String statementTag;
@@ -271,6 +272,7 @@ static UnitOfWorkType of(TransactionMode transactionMode) {
271272
this.autocommit = options.isAutocommit();
272273
this.queryOptions = this.queryOptions.toBuilder().mergeFrom(options.getQueryOptions()).build();
273274
this.rpcPriority = options.getRPCPriority();
275+
this.ddlInTransactionMode = options.getDdlInTransactionMode();
274276
this.returnCommitStats = options.isReturnCommitStats();
275277
this.delayTransactionStartUntilFirstWrite = options.isDelayTransactionStartUntilFirstWrite();
276278
this.dataBoostEnabled = options.isDataBoostEnabled();
@@ -296,6 +298,7 @@ static UnitOfWorkType of(TransactionMode transactionMode) {
296298
new StatementExecutor(options.isUseVirtualThreads(), Collections.emptyList());
297299
this.spannerPool = Preconditions.checkNotNull(spannerPool);
298300
this.options = Preconditions.checkNotNull(options);
301+
this.ddlInTransactionMode = options.getDdlInTransactionMode();
299302
this.spanner = spannerPool.getSpanner(options, this);
300303
this.ddlClient = Preconditions.checkNotNull(ddlClient);
301304
this.dbClient = Preconditions.checkNotNull(dbClient);
@@ -571,6 +574,21 @@ public RpcPriority getRPCPriority() {
571574
return this.rpcPriority;
572575
}
573576

577+
@Override
578+
public DdlInTransactionMode getDdlInTransactionMode() {
579+
return this.ddlInTransactionMode;
580+
}
581+
582+
@Override
583+
public void setDdlInTransactionMode(DdlInTransactionMode ddlInTransactionMode) {
584+
ConnectionPreconditions.checkState(!isClosed(), CLOSED_ERROR_MSG);
585+
ConnectionPreconditions.checkState(
586+
!isBatchActive(), "Cannot set DdlInTransactionMode while in a batch");
587+
ConnectionPreconditions.checkState(
588+
!isTransactionStarted(), "Cannot set DdlInTransactionMode while a transaction is active");
589+
this.ddlInTransactionMode = Preconditions.checkNotNull(ddlInTransactionMode);
590+
}
591+
574592
@Override
575593
public void setStatementTimeout(long timeout, TimeUnit unit) {
576594
Preconditions.checkArgument(timeout > 0L, "Zero or negative timeout values are not allowed");
@@ -1639,28 +1657,55 @@ private ApiFuture<long[]> internalExecuteBatchUpdateAsync(
16391657
}
16401658

16411659
private UnitOfWork getCurrentUnitOfWorkOrStartNewUnitOfWork() {
1642-
return getCurrentUnitOfWorkOrStartNewUnitOfWork(false);
1660+
return getCurrentUnitOfWorkOrStartNewUnitOfWork(StatementType.UNKNOWN, false);
1661+
}
1662+
1663+
@VisibleForTesting
1664+
UnitOfWork getCurrentUnitOfWorkOrStartNewUnitOfWork(boolean isInternalMetadataQuery) {
1665+
return getCurrentUnitOfWorkOrStartNewUnitOfWork(StatementType.UNKNOWN, isInternalMetadataQuery);
1666+
}
1667+
1668+
private UnitOfWork getOrStartDdlUnitOfWork() {
1669+
return getCurrentUnitOfWorkOrStartNewUnitOfWork(StatementType.DDL, false);
16431670
}
16441671

16451672
/**
16461673
* Returns the current {@link UnitOfWork} of this connection, or creates a new one based on the
16471674
* current transaction settings of the connection and returns that.
16481675
*/
16491676
@VisibleForTesting
1650-
UnitOfWork getCurrentUnitOfWorkOrStartNewUnitOfWork(boolean isInternalMetadataQuery) {
1677+
UnitOfWork getCurrentUnitOfWorkOrStartNewUnitOfWork(
1678+
StatementType statementType, boolean isInternalMetadataQuery) {
16511679
if (isInternalMetadataQuery) {
16521680
// Just return a temporary single-use transaction.
1653-
return createNewUnitOfWork(true);
1681+
return createNewUnitOfWork(/* isInternalMetadataQuery = */ true, /* forceSingleUse = */ true);
16541682
}
1683+
maybeAutoCommitCurrentTransaction(statementType);
16551684
if (this.currentUnitOfWork == null || !this.currentUnitOfWork.isActive()) {
1656-
this.currentUnitOfWork = createNewUnitOfWork(false);
1685+
this.currentUnitOfWork =
1686+
createNewUnitOfWork(
1687+
/* isInternalMetadataQuery = */ false,
1688+
/* forceSingleUse = */ statementType == StatementType.DDL
1689+
&& this.ddlInTransactionMode != DdlInTransactionMode.FAIL
1690+
&& !this.transactionBeginMarked);
16571691
}
16581692
return this.currentUnitOfWork;
16591693
}
16601694

1695+
void maybeAutoCommitCurrentTransaction(StatementType statementType) {
1696+
if (this.currentUnitOfWork instanceof ReadWriteTransaction
1697+
&& this.currentUnitOfWork.isActive()
1698+
&& statementType == StatementType.DDL
1699+
&& this.ddlInTransactionMode == DdlInTransactionMode.AUTO_COMMIT_TRANSACTION) {
1700+
commit();
1701+
}
1702+
}
1703+
16611704
@VisibleForTesting
1662-
UnitOfWork createNewUnitOfWork(boolean isInternalMetadataQuery) {
1663-
if (isInternalMetadataQuery || (isAutocommit() && !isInTransaction() && !isInBatch())) {
1705+
UnitOfWork createNewUnitOfWork(boolean isInternalMetadataQuery, boolean forceSingleUse) {
1706+
if (isInternalMetadataQuery
1707+
|| (isAutocommit() && !isInTransaction() && !isInBatch())
1708+
|| forceSingleUse) {
16641709
return SingleUseTransaction.newBuilder()
16651710
.setInternalMetadataQuery(isInternalMetadataQuery)
16661711
.setDdlClient(ddlClient)
@@ -1741,7 +1786,7 @@ private void popUnitOfWorkFromTransactionStack() {
17411786
}
17421787

17431788
private ApiFuture<Void> executeDdlAsync(CallType callType, ParsedStatement ddl) {
1744-
return getCurrentUnitOfWorkOrStartNewUnitOfWork().executeDdlAsync(callType, ddl);
1789+
return getOrStartDdlUnitOfWork().executeDdlAsync(callType, ddl);
17451790
}
17461791

17471792
@Override
@@ -1788,15 +1833,23 @@ public void startBatchDdl() {
17881833
ConnectionPreconditions.checkState(
17891834
!isReadOnly(), "Cannot start a DDL batch when the connection is in read-only mode");
17901835
ConnectionPreconditions.checkState(
1791-
!isTransactionStarted(), "Cannot start a DDL batch while a transaction is active");
1836+
!isTransactionStarted()
1837+
|| getDdlInTransactionMode() == DdlInTransactionMode.AUTO_COMMIT_TRANSACTION,
1838+
"Cannot start a DDL batch while a transaction is active");
17921839
ConnectionPreconditions.checkState(
17931840
!(isAutocommit() && isInTransaction()),
17941841
"Cannot start a DDL batch while in a temporary transaction");
17951842
ConnectionPreconditions.checkState(
17961843
!transactionBeginMarked, "Cannot start a DDL batch when a transaction has begun");
1844+
ConnectionPreconditions.checkState(
1845+
isAutocommit() || getDdlInTransactionMode() != DdlInTransactionMode.FAIL,
1846+
"Cannot start a DDL batch when autocommit=false and ddlInTransactionMode=FAIL");
1847+
1848+
maybeAutoCommitCurrentTransaction(StatementType.DDL);
17971849
this.batchMode = BatchMode.DDL;
17981850
this.unitOfWorkType = UnitOfWorkType.DDL_BATCH;
1799-
this.currentUnitOfWork = createNewUnitOfWork(false);
1851+
this.currentUnitOfWork =
1852+
createNewUnitOfWork(/* isInternalMetadataQuery = */ false, /* forceSingleUse = */ false);
18001853
}
18011854

18021855
@Override
@@ -1814,7 +1867,8 @@ public void startBatchDml() {
18141867
// Then create the DML batch.
18151868
this.batchMode = BatchMode.DML;
18161869
this.unitOfWorkType = UnitOfWorkType.DML_BATCH;
1817-
this.currentUnitOfWork = createNewUnitOfWork(false);
1870+
this.currentUnitOfWork =
1871+
createNewUnitOfWork(/* isInternalMetadataQuery = */ false, /* forceSingleUse = */ false);
18181872
}
18191873

18201874
@Override

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

+23
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,8 @@ public String[] getValidValues() {
183183
private static final String DEFAULT_OPTIMIZER_VERSION = "";
184184
private static final String DEFAULT_OPTIMIZER_STATISTICS_PACKAGE = "";
185185
private static final RpcPriority DEFAULT_RPC_PRIORITY = null;
186+
private static final DdlInTransactionMode DEFAULT_DDL_IN_TRANSACTION_MODE =
187+
DdlInTransactionMode.ALLOW_IN_EMPTY_TRANSACTION;
186188
private static final boolean DEFAULT_RETURN_COMMIT_STATS = false;
187189
private static final boolean DEFAULT_LENIENT = false;
188190
private static final boolean DEFAULT_ROUTE_TO_LEADER = true;
@@ -253,6 +255,8 @@ public String[] getValidValues() {
253255
public static final String LENIENT_PROPERTY_NAME = "lenient";
254256
/** Name of the 'rpcPriority' connection property. */
255257
public static final String RPC_PRIORITY_NAME = "rpcPriority";
258+
259+
public static final String DDL_IN_TRANSACTION_MODE_PROPERTY_NAME = "ddlInTransactionMode";
256260
/** Dialect to use for a connection. */
257261
private static final String DIALECT_PROPERTY_NAME = "dialect";
258262
/** Name of the 'databaseRole' connection property. */
@@ -374,6 +378,11 @@ private static String generateGuardedConnectionPropertyError(
374378
ConnectionProperty.createStringProperty(
375379
RPC_PRIORITY_NAME,
376380
"Sets the priority for all RPC invocations from this connection (HIGH/MEDIUM/LOW). The default is HIGH."),
381+
ConnectionProperty.createStringProperty(
382+
DDL_IN_TRANSACTION_MODE_PROPERTY_NAME,
383+
"Sets the behavior of a connection when a DDL statement is executed in a read/write transaction. The default is "
384+
+ DEFAULT_DDL_IN_TRANSACTION_MODE
385+
+ "."),
377386
ConnectionProperty.createStringProperty(
378387
DIALECT_PROPERTY_NAME,
379388
"Sets the dialect to use for new databases that are created by this connection."),
@@ -697,6 +706,7 @@ public static Builder newBuilder() {
697706
private final boolean autoConfigEmulator;
698707
private final Dialect dialect;
699708
private final RpcPriority rpcPriority;
709+
private final DdlInTransactionMode ddlInTransactionMode;
700710
private final boolean delayTransactionStartUntilFirstWrite;
701711
private final boolean trackSessionLeaks;
702712
private final boolean trackConnectionLeaks;
@@ -757,6 +767,7 @@ private ConnectionOptions(Builder builder) {
757767
determineHost(
758768
matcher, parseEndpoint(this.uri), autoConfigEmulator, usePlainText, System.getenv());
759769
this.rpcPriority = parseRPCPriority(this.uri);
770+
this.ddlInTransactionMode = parseDdlInTransactionMode(this.uri);
760771
this.delayTransactionStartUntilFirstWrite = parseDelayTransactionStartUntilFirstWrite(this.uri);
761772
this.trackSessionLeaks = parseTrackSessionLeaks(this.uri);
762773
this.trackConnectionLeaks = parseTrackConnectionLeaks(this.uri);
@@ -1195,6 +1206,14 @@ static RpcPriority parseRPCPriority(String uri) {
11951206
return value != null ? RpcPriority.valueOf(value) : DEFAULT_RPC_PRIORITY;
11961207
}
11971208

1209+
@VisibleForTesting
1210+
static DdlInTransactionMode parseDdlInTransactionMode(String uri) {
1211+
String value = parseUriProperty(uri, DDL_IN_TRANSACTION_MODE_PROPERTY_NAME);
1212+
return value != null
1213+
? DdlInTransactionMode.valueOf(value.toUpperCase())
1214+
: DEFAULT_DDL_IN_TRANSACTION_MODE;
1215+
}
1216+
11981217
@VisibleForTesting
11991218
static String parseUriProperty(String uri, String property) {
12001219
Pattern pattern = Pattern.compile(String.format("(?is)(?:;|\\?)%s=(.*?)(?:;|$)", property));
@@ -1466,6 +1485,10 @@ RpcPriority getRPCPriority() {
14661485
return rpcPriority;
14671486
}
14681487

1488+
DdlInTransactionMode getDdlInTransactionMode() {
1489+
return this.ddlInTransactionMode;
1490+
}
1491+
14691492
/**
14701493
* Whether connections created by this {@link ConnectionOptions} should delay the actual start of
14711494
* a read/write transaction until the first write operation.

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

+35
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/*
2+
* Copyright 2024 Google LLC
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.google.cloud.spanner.connection;
18+
19+
/** Enum used for setting the behavior of DDL in read/write transactions. */
20+
public enum DdlInTransactionMode {
21+
/** All DDL statements in a read/write transaction fail. */
22+
FAIL,
23+
/**
24+
* DDL statements in an empty transaction are allowed. That is; if the connection is in
25+
* AutoCommit=false mode and no other statement has been executed, then executing a DDL statement
26+
* or a DDL batch is allowed.
27+
*/
28+
ALLOW_IN_EMPTY_TRANSACTION,
29+
/**
30+
* DDL statements automatically cause the current transaction to be committed and the DDL
31+
* statement is subsequently executed without a transaction. This is equal to how MySQL and Oracle
32+
* behave.
33+
*/
34+
AUTO_COMMIT_TRANSACTION;
35+
}

‎google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionImplTest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -1751,7 +1751,7 @@ public void testTransactionTagNotAllowedAfterTransactionStarted() {
17511751
new ConnectionImpl(
17521752
connectionOptions, spannerPool, ddlClient, dbClient, mock(BatchClient.class)) {
17531753
@Override
1754-
UnitOfWork createNewUnitOfWork(boolean isInternalMetadataQuery) {
1754+
UnitOfWork createNewUnitOfWork(boolean isInternalMetadataQuery, boolean forceSingleUse) {
17551755
return unitOfWork;
17561756
}
17571757
}) {

‎google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionImplTransactionalReadWriteTest.java

+14-7
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,8 @@ boolean isGetCommitTimestampAllowed() {
151151
boolean isExecuteAllowed(StatementType type) {
152152
return type == StatementType.CLIENT_SIDE
153153
|| type == StatementType.QUERY
154-
|| type == StatementType.UPDATE;
154+
|| type == StatementType.UPDATE
155+
|| type == StatementType.DDL;
155156
}
156157

157158
@Override
@@ -765,7 +766,8 @@ boolean isGetCommitTimestampAllowed() {
765766
boolean isExecuteAllowed(StatementType type) {
766767
return type == StatementType.CLIENT_SIDE
767768
|| type == StatementType.QUERY
768-
|| type == StatementType.UPDATE;
769+
|| type == StatementType.UPDATE
770+
|| type == StatementType.DDL;
769771
}
770772

771773
@Override
@@ -920,7 +922,8 @@ boolean isGetCommitTimestampAllowed() {
920922
boolean isExecuteAllowed(StatementType type) {
921923
return type == StatementType.CLIENT_SIDE
922924
|| type == StatementType.QUERY
923-
|| type == StatementType.UPDATE;
925+
|| type == StatementType.UPDATE
926+
|| type == StatementType.DDL;
924927
}
925928

926929
@Override
@@ -1074,7 +1077,8 @@ boolean isGetCommitTimestampAllowed() {
10741077
boolean isExecuteAllowed(StatementType type) {
10751078
return type == StatementType.CLIENT_SIDE
10761079
|| type == StatementType.QUERY
1077-
|| type == StatementType.UPDATE;
1080+
|| type == StatementType.UPDATE
1081+
|| type == StatementType.DDL;
10781082
}
10791083

10801084
@Override
@@ -1378,7 +1382,8 @@ boolean isGetCommitTimestampAllowed() {
13781382
boolean isExecuteAllowed(StatementType type) {
13791383
return type == StatementType.CLIENT_SIDE
13801384
|| type == StatementType.QUERY
1381-
|| type == StatementType.UPDATE;
1385+
|| type == StatementType.UPDATE
1386+
|| type == StatementType.DDL;
13821387
}
13831388

13841389
@Override
@@ -1829,7 +1834,8 @@ boolean isGetCommitTimestampAllowed() {
18291834
boolean isExecuteAllowed(StatementType type) {
18301835
return type == StatementType.CLIENT_SIDE
18311836
|| type == StatementType.QUERY
1832-
|| type == StatementType.UPDATE;
1837+
|| type == StatementType.UPDATE
1838+
|| type == StatementType.DDL;
18331839
}
18341840

18351841
@Override
@@ -1979,7 +1985,8 @@ boolean isGetCommitTimestampAllowed() {
19791985
boolean isExecuteAllowed(StatementType type) {
19801986
return type == StatementType.CLIENT_SIDE
19811987
|| type == StatementType.QUERY
1982-
|| type == StatementType.UPDATE;
1988+
|| type == StatementType.UPDATE
1989+
|| type == StatementType.DDL;
19831990
}
19841991

19851992
@Override

0 commit comments

Comments
 (0)