Skip to content

Commit 3fab588

Browse files
vlsivlsi
committed
test: add autosave=always|never|conservative and cleanupSavepoints=true|false to the randomized CI jobs
1 parent 9c20cc2 commit 3fab588

4 files changed

Lines changed: 72 additions & 7 deletions

File tree

‎.github/workflows/matrix.mjs‎

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,25 @@ matrix.addAxis({
189189
]
190190
});
191191

192+
matrix.addAxis({
193+
name: 'autosave',
194+
title: x => x.value === 'never' ? '' : 'autosave ' + x.value,
195+
values: [
196+
{value: 'always', weight: 5},
197+
{value: 'never', weight: 30},
198+
{value: 'conservative', weight: 5},
199+
]
200+
});
201+
202+
matrix.addAxis({
203+
name: 'cleanupSavepoints',
204+
title: x => x.value === 'true' ? 'cleanupSavepoints' : '',
205+
values: [
206+
{value: 'true', weight: 5},
207+
{value: 'false', weight: 5},
208+
]
209+
});
210+
192211
matrix.addAxis({
193212
name: 'check_anorm_sbt',
194213
values: [
@@ -233,7 +252,8 @@ matrix.setNamePattern([
233252
'java_version', 'java_distribution', 'pg_version', 'query_mode', 'scram', 'ssl', 'hash', 'os',
234253
'server_tz', 'tz', 'locale',
235254
'check_anorm_sbt', 'gss', 'replication', 'slow_tests',
236-
'adaptive_fetch', 'rewrite_batch_inserts', 'query_timeout'
255+
'adaptive_fetch', 'rewrite_batch_inserts', 'query_timeout',
256+
'autosave', 'cleanupSavepoints'
237257
]);
238258

239259
// We take EA builds from Oracle
@@ -252,6 +272,8 @@ matrix.imply({java_distribution: {value: 'oracle'}}, {java_version: v => v === e
252272
// TODO: Semeru does not ship Java 21 builds yet
253273
matrix.exclude({java_distribution: {value: 'semeru'}, java_version: '21'})
254274
matrix.exclude({gss: {value: 'yes'}, os: ['windows-latest', 'macos-latest']})
275+
// cleanupSavepoints is not relevant when autosave=never
276+
matrix.imply({autosave: {value: 'never'}}, {cleanupSavepoints: {value: 'false'}});
255277

256278
// The most rare features should be generated the first
257279
// For instance, we have a lot of PostgreSQL versions, so we generate the minimal the first
@@ -292,6 +314,8 @@ for (let ssl of matrix.axisByName.ssl.values) {
292314
for (let replication of matrix.axisByName.replication.values) {
293315
matrix.generateRow({replication: replication});
294316
}
317+
// Ensure at least one job with autosave=always
318+
matrix.generateRow({autosave: {value: 'always'}});
295319
const include = matrix.generateRows(process.env.MATRIX_JOBS || 5);
296320
if (include.length === 0) {
297321
throw new Error('Matrix list is empty');
@@ -337,6 +361,8 @@ include.forEach(v => {
337361
v.adaptive_fetch = v.adaptive_fetch.value;
338362
v.rewrite_batch_inserts = v.rewrite_batch_inserts.value;
339363
v.query_timeout = v.query_timeout.value;
364+
v.autosave = v.autosave.value;
365+
v.cleanupSavepoints = v.cleanupSavepoints.value;
340366

341367
let includeTestTags = [];
342368
// See https://junit.org/junit5/docs/current/user-guide/#running-tests-tag-expressions
@@ -405,6 +431,12 @@ include.forEach(v => {
405431
if (v.query_timeout) {
406432
testJvmArgs.push(`-DqueryTimeout=${v.query_timeout}`);
407433
}
434+
if (v.autosave !== 'never') {
435+
testJvmArgs.push(`-Dautosave=${v.autosave}`);
436+
}
437+
if (v.cleanupSavepoints === 'true') {
438+
testJvmArgs.push('-DcleanupSavepoints=true');
439+
}
408440
v.extraJvmArgs = jvmArgs.join(' ');
409441
v.testExtraJvmArgs = testJvmArgs.join(' ::: ');
410442
delete v.hash;

‎build-logic/jvm/src/main/kotlin/build-logic.test-base.gradle.kts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ tasks.configureEach<Test> {
4646
}
4747
for (p in listOf("test.url.PGHOST", "test.url.PGPORT", "test.url.PGDBNAME", "user", "password",
4848
"privilegedUser", "privilegedPassword",
49-
"simpleProtocolOnly", "enable_ssl_tests")) {
49+
"simpleProtocolOnly", "enable_ssl_tests",
50+
"autosave", "cleanupSavepoints")) {
5051
passProperty(p)
5152
}
5253
}

‎pgjdbc/src/main/java/org/postgresql/core/v3/QueryExecutorImpl.java‎

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -478,13 +478,13 @@ private boolean sendAutomaticSavepoint(Query query, int flags) throws IOExceptio
478478
}
479479

480480
private boolean shouldCreateAutomaticSavepoint(Query query, int flags) {
481-
if (!isTransactionActive(flags)) {
481+
if (getAutoSave() == AutoSave.NEVER) {
482482
return false;
483483
}
484-
if (isSpecialQuery(query)) {
484+
if (!isTransactionActive(flags)) {
485485
return false;
486486
}
487-
if (getAutoSave() == AutoSave.NEVER) {
487+
if (isSpecialQuery(query)) {
488488
return false;
489489
}
490490
return getAutoSave() == AutoSave.ALWAYS || queryMightFail(query);
@@ -496,8 +496,25 @@ private boolean isTransactionActive(int flags) {
496496
}
497497

498498
private boolean isSpecialQuery(Query query) {
499-
return query == restoreToAutoSave
500-
|| "COMMIT".equalsIgnoreCase(query.getNativeSql());
499+
if (query == restoreToAutoSave) {
500+
return true;
501+
}
502+
String sql = query.getNativeSql();
503+
// SET TRANSACTION ISOLATION LEVEL and SET SESSION CHARACTERISTICS cannot be called in subtransaction
504+
// SAVEPOINT commands cannot use autosave because:
505+
// - SAVEPOINT: releasing the autosave would destroy the user's savepoint (created after autosave)
506+
// - RELEASE SAVEPOINT: same issue, plus the released savepoint might no longer exist
507+
// - ROLLBACK TO SAVEPOINT: destroys savepoints created after the target, including autosave
508+
return "COMMIT".equalsIgnoreCase(sql)
509+
|| startsWithIgnoreCase(sql, "SET TRANSACTION")
510+
|| startsWithIgnoreCase(sql, "SET SESSION CHARACTERISTICS")
511+
|| startsWithIgnoreCase(sql, "SAVEPOINT")
512+
|| startsWithIgnoreCase(sql, "RELEASE")
513+
|| startsWithIgnoreCase(sql, "ROLLBACK TO SAVEPOINT");
514+
}
515+
516+
private static boolean startsWithIgnoreCase(String str, String prefix) {
517+
return str.regionMatches(true, 0, prefix, 0, prefix.length());
501518
}
502519

503520
// If query has no resulting fields, it cannot fail with 'cached plan must not change result type'
@@ -1051,6 +1068,12 @@ public CopyOperation startCopy(String sql, boolean suppressBegin)
10511068
byte[] buf = sql.getBytes(StandardCharsets.UTF_8);
10521069

10531070
try {
1071+
// Process any pending responses from simple queries (e.g., RELEASE SAVEPOINT
1072+
// from cleanupSavepoints). These responses would otherwise be misinterpreted
1073+
// by processCopyResults(). See https://github.com/pgjdbc/pgjdbc/issues/3910
1074+
if (!pendingExecuteQueue.isEmpty()) {
1075+
processResults(new ResultHandlerBase(), 0);
1076+
}
10541077
LOGGER.log(Level.FINEST, " FE=> Query(CopyStart)");
10551078

10561079
pgStream.sendChar(PgMessageType.QUERY_REQUEST);

‎pgjdbc/src/test/java/org/postgresql/jdbc/LargeObjectManagerTest.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,20 @@ class LargeObjectManagerTest {
3838
* It is possible for PostgreSQL to send a ParameterStatus message after an ErrorResponse
3939
* Receiving such a message should not lead to an invalid connection state
4040
* See https://github.com/pgjdbc/pgjdbc/issues/2237
41+
*
42+
* Note: This test is skipped when autosave is enabled because with autosave, a savepoint
43+
* is created before the SET statement. When a subsequent error occurs, PostgreSQL doesn't
44+
* send ParameterStatus to reset the parameter because it expects the client might
45+
* ROLLBACK TO SAVEPOINT to recover. This is expected autosave behavior - protecting
46+
* previous successful statements from being rolled back.
4147
*/
4248
@Test
4349
void openWithErrorAndSubsequentParameterStatusMessageShouldLeaveConnectionInUsableStateAndUpdateParameterStatus() throws Exception {
4450
try (PgConnection con = (PgConnection) TestUtil.openDB()) {
4551
assumeTrue(TestUtil.haveMinimumServerVersion(con, ServerVersion.v9_0));
52+
assumeTrue(con.getAutosave() == AutoSave.NEVER,
53+
"Test requires autosave=never because autosave creates savepoints that prevent "
54+
+ "PostgreSQL from sending ParameterStatus on error");
4655
con.setAutoCommit(false);
4756
String originalApplicationName = con.getParameterStatus("application_name");
4857
try (Statement statement = con.createStatement()) {

0 commit comments

Comments
 (0)