🌐 US-Proxy
class="logged-out env-production page-responsive" style="word-wrap: break-word;" >
Skip to content
This repository was archived by the owner on Aug 30, 2026. It is now read-only.

Commit 43ea4fa

Browse files
olavloiteolavloite
authored
fix: allow zero durations to be set for connections (#3916)
SET statements that set a duration, e.g. max_commit_delay or statement_timeout, did not allow setting a zero duration. This is however sometimes useful, as the server default for max_commit_delay is not zero. This means that setting this to zero triggers a different behavior than not setting a value, as when no value is included in a commit request, Spanner will choose a reasonable value for the commit delay. By explicitly setting it to zero, Spanner will always also use a zero commit delay.
1 parent 1dc5a3e commit 43ea4fa

8 files changed

Lines changed: 277 additions & 526 deletions

File tree

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,8 @@ public Duration convert(String value) {
248248
} else {
249249
duration = Duration.ofMillis(Long.parseLong(value.trim()));
250250
}
251-
if (duration.isZero()) {
251+
// Converters should return null for invalid values.
252+
if (duration.isNegative()) {
252253
return null;
253254
}
254255
return duration;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,7 @@ public StatementResult statementShowReturnCommitStats() {
361361

362362
@Override
363363
public StatementResult statementSetMaxCommitDelay(Duration duration) {
364-
getConnection().setMaxCommitDelay(duration == null || duration.isZero() ? null : duration);
364+
getConnection().setMaxCommitDelay(duration);
365365
return noResult(SET_MAX_COMMIT_DELAY);
366366
}
367367

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -276,13 +276,14 @@ public void testSetStatementTimeout() {
276276
assertThat(connection.hasStatementTimeout(), is(false));
277277
boolean gotException = false;
278278
try {
279-
log("@EXPECT EXCEPTION INVALID_ARGUMENT");
279+
// log("@EXPECT EXCEPTION INVALID_ARGUMENT");
280280
log(String.format("SET STATEMENT_TIMEOUT='0%s';", getTimeUnitAbbreviation(unit)));
281-
connection.setStatementTimeout(0L, unit);
281+
connection.clearStatementTimeout();
282+
// connection.setStatementTimeout(0L, unit);
282283
} catch (IllegalArgumentException e) {
283284
gotException = true;
284285
}
285-
assertThat(gotException, is(true));
286+
assertThat(gotException, is(false));
286287
log(
287288
String.format(
288289
"@EXPECT RESULT_SET 'STATEMENT_TIMEOUT',%s",

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public void testConvert() throws CompileException {
4242
DurationConverter converter = new DurationConverter(allowedValues);
4343
assertThat(converter.convert("'100ms'"), is(equalTo(Duration.ofMillis(100L))));
4444
assertThat(converter.convert("100"), is(equalTo(Duration.ofMillis(100))));
45-
assertThat(converter.convert("'0ms'"), is(nullValue()));
45+
assertThat(converter.convert("'0ms'"), is(Duration.ZERO));
4646
assertThat(converter.convert("'-100ms'"), is(nullValue()));
4747
assertThat(
4848
converter.convert("'315576000000000ms'"), is(equalTo(Duration.ofSeconds(315576000000L))));

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

Lines changed: 44 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,20 @@ public void testNoMaxCommitDelayByDefault() {
8383
for (boolean autocommit : new boolean[] {true, false}) {
8484
connection.setAutocommit(autocommit);
8585
executeCommit(connection);
86-
assertMaxCommitDelay(Duration.getDefaultInstance());
86+
assertMaxCommitDelay(Duration.getDefaultInstance(), false);
87+
mockSpanner.clearRequests();
88+
}
89+
}
90+
}
91+
92+
@Test
93+
public void testZeroMaxCommitDelay() {
94+
try (Connection connection = createConnection()) {
95+
for (boolean autocommit : new boolean[] {true, false}) {
96+
connection.setAutocommit(autocommit);
97+
connection.setMaxCommitDelay(java.time.Duration.ZERO);
98+
executeCommit(connection);
99+
assertMaxCommitDelay(Duration.getDefaultInstance(), true);
87100
mockSpanner.clearRequests();
88101
}
89102
}
@@ -95,7 +108,19 @@ public void testMaxCommitDelayInConnectionString() {
95108
for (boolean autocommit : new boolean[] {true, false}) {
96109
connection.setAutocommit(autocommit);
97110
executeCommit(connection);
98-
assertMaxCommitDelay(Duration.newBuilder().setSeconds(1).build());
111+
assertMaxCommitDelay(Duration.newBuilder().setSeconds(1).build(), true);
112+
mockSpanner.clearRequests();
113+
}
114+
}
115+
}
116+
117+
@Test
118+
public void testZeroMaxCommitDelayInConnectionString() {
119+
try (Connection connection = createConnection(";maxCommitDelay=0")) {
120+
for (boolean autocommit : new boolean[] {true, false}) {
121+
connection.setAutocommit(autocommit);
122+
executeCommit(connection);
123+
assertMaxCommitDelay(Duration.getDefaultInstance(), true);
99124
mockSpanner.clearRequests();
100125
}
101126
}
@@ -121,20 +146,31 @@ public void testSetMaxCommitDelay() {
121146
() -> {
122147
executeCommit(connection);
123148
assertMaxCommitDelay(
124-
Duration.newBuilder()
125-
.setNanos((int) TimeUnit.MILLISECONDS.toNanos(40))
126-
.build());
149+
Duration.newBuilder().setNanos((int) TimeUnit.MILLISECONDS.toNanos(40)).build(),
150+
true);
127151
mockSpanner.clearRequests();
128152
});
129153

130154
if (useSql) {
155+
// This is translated to Duration.ZERO.
131156
connection.execute(
132157
Statement.of(String.format("set %smax_commit_delay=null", getVariablePrefix())));
133158
} else {
134159
connection.setMaxCommitDelay(null);
135160
}
136161
executeCommit(connection);
137-
assertMaxCommitDelay(Duration.getDefaultInstance());
162+
// The SQL statement set max_commit_delay=null is translated to Duration.ZERO.
163+
assertMaxCommitDelay(Duration.getDefaultInstance(), useSql);
164+
mockSpanner.clearRequests();
165+
166+
if (useSql) {
167+
connection.execute(
168+
Statement.of(String.format("set %smax_commit_delay=0", getVariablePrefix())));
169+
} else {
170+
connection.setMaxCommitDelay(java.time.Duration.ZERO);
171+
}
172+
executeCommit(connection);
173+
assertMaxCommitDelay(Duration.getDefaultInstance(), true);
138174
mockSpanner.clearRequests();
139175
}
140176
}
@@ -150,10 +186,11 @@ void executeCommit(Connection connection) {
150186
}
151187
}
152188

153-
private void assertMaxCommitDelay(Duration expected) {
189+
private void assertMaxCommitDelay(Duration expected, boolean hasMaxCommitDelay) {
154190
List<CommitRequest> requests = mockSpanner.getRequestsOfType(CommitRequest.class);
155191
assertEquals(1, requests.size());
156192
CommitRequest request = requests.get(0);
157193
assertEquals(expected, request.getMaxCommitDelay());
194+
assertEquals(hasMaxCommitDelay, request.hasMaxCommitDelay());
158195
}
159196
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public void testConvert() throws CompileException {
4545

4646
assertEquals(
4747
Duration.ofNanos((int) TimeUnit.MILLISECONDS.toNanos(100L)), converter.convert("'100ms'"));
48-
assertNull(converter.convert("'0ms'"));
48+
assertEquals(Duration.ZERO, converter.convert("'0ms'"));
4949
assertNull(converter.convert("'-100ms'"));
5050
assertEquals(Duration.ofSeconds(315576000000L), converter.convert("'315576000000000ms'"));
5151
assertEquals(Duration.ofSeconds(1L), converter.convert("'1s'"));

0 commit comments

Comments
 (0)