From 20fc74219a483cfc048fa57a463375fd8284342f Mon Sep 17 00:00:00 2001 From: rob bygrave Date: Tue, 24 Nov 2020 23:58:58 +1300 Subject: [PATCH] #2089 - Postgres - Use NO KEY with FOR UPDATE clauses with Postgres Rename PlatformConfig.defaultLockWithKey to PlatformConfig.forUpdateNoKey --- .../java/io/ebean/config/PlatformConfig.java | 20 +++++++++---------- .../dbplatform/postgres/PostgresPlatform.java | 6 +++--- .../server/expression/JunctionExpression.java | 15 ++++++++++++++ .../io/ebean/config/ServerConfigTest.java | 4 ++-- .../dbplatform/PostgresPlatformTest.java | 6 +++--- 5 files changed, 33 insertions(+), 18 deletions(-) diff --git a/ebean-api/src/main/java/io/ebean/config/PlatformConfig.java b/ebean-api/src/main/java/io/ebean/config/PlatformConfig.java index dff98b1f9..bcb97158d 100644 --- a/ebean-api/src/main/java/io/ebean/config/PlatformConfig.java +++ b/ebean-api/src/main/java/io/ebean/config/PlatformConfig.java @@ -18,9 +18,9 @@ public class PlatformConfig { private boolean allQuotedIdentifiers; /** - * Set this to true for Postgres FOR UPDATE to include the primary key (not use NO KEY). + * Set this to true for Postgres FOR UPDATE to use NO KEY option. */ - private boolean lockWithKey = true; + private boolean forUpdateNoKey; private DbConstraintNaming constraintNaming; @@ -82,7 +82,7 @@ public class PlatformConfig { * Construct based on given config - typically for DbMigration generation with many platforms. */ public PlatformConfig(PlatformConfig platformConfig) { - this.lockWithKey = platformConfig.lockWithKey; + this.forUpdateNoKey = platformConfig.forUpdateNoKey; this.databaseBooleanFalse = platformConfig.databaseBooleanFalse; this.databaseBooleanTrue = platformConfig.databaseBooleanTrue; this.databaseSequenceBatchSize = platformConfig.databaseSequenceBatchSize; @@ -140,17 +140,17 @@ public class PlatformConfig { } /** - * Return true if Postgres FOR UPDATE should include the primary key (or use NO KEY). + * Return true if Postgres FOR UPDATE should use the NO KEY option. */ - public boolean isLockWithKey() { - return lockWithKey; + public boolean isForUpdateNoKey() { + return forUpdateNoKey; } /** - * Set to true such that Postgres FOR UPDATE should include the primary key (not use NO KEY option). + * Set to true such that Postgres FOR UPDATE should use the NO KEY option. */ - public void setLockWithKey(boolean lockWithKey) { - this.lockWithKey = lockWithKey; + public void setForUpdateNoKey(boolean forUpdateNoKey) { + this.forUpdateNoKey = forUpdateNoKey; } /** @@ -308,7 +308,7 @@ public class PlatformConfig { public void loadSettings(PropertiesWrapper p) { idType = p.getEnum(IdType.class, "idType", idType); - lockWithKey = p.getBoolean("lockWithKey", lockWithKey); + forUpdateNoKey = p.getBoolean("forUpdateNoKey", forUpdateNoKey); databaseSequenceBatchSize = p.getInt("databaseSequenceBatchSize", databaseSequenceBatchSize); databaseBooleanTrue = p.get("databaseBooleanTrue", databaseBooleanTrue); databaseBooleanFalse = p.get("databaseBooleanFalse", databaseBooleanFalse); diff --git a/ebean-api/src/main/java/io/ebean/config/dbplatform/postgres/PostgresPlatform.java b/ebean-api/src/main/java/io/ebean/config/dbplatform/postgres/PostgresPlatform.java index 247f1f28e..9a5ba9d3d 100644 --- a/ebean-api/src/main/java/io/ebean/config/dbplatform/postgres/PostgresPlatform.java +++ b/ebean-api/src/main/java/io/ebean/config/dbplatform/postgres/PostgresPlatform.java @@ -33,7 +33,7 @@ public class PostgresPlatform extends DatabasePlatform { private static final String FOR_SHARE = " for share"; private static final String FOR_KEY_SHARE = " for key share"; - private boolean defaultLockWithKey = true; + private boolean forUpdateNoKey; public PostgresPlatform() { super(); @@ -94,7 +94,7 @@ public class PostgresPlatform extends DatabasePlatform { @Override public void configure(PlatformConfig config) { super.configure(config); - defaultLockWithKey = config.isLockWithKey(); + forUpdateNoKey = config.isForUpdateNoKey(); } @Override @@ -145,7 +145,7 @@ public class PostgresPlatform extends DatabasePlatform { case NoKeyUpdate: return FOR_NO_KEY_UPDATE; case Share: return FOR_SHARE; case KeyShare: return FOR_KEY_SHARE; - case Default: return defaultLockWithKey ? FOR_UPDATE : FOR_NO_KEY_UPDATE; + case Default: return forUpdateNoKey ? FOR_NO_KEY_UPDATE : FOR_UPDATE; } return FOR_UPDATE; } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/expression/JunctionExpression.java b/ebean-core/src/main/java/io/ebeaninternal/server/expression/JunctionExpression.java index 8a043c61d..37dab58f9 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/expression/JunctionExpression.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/expression/JunctionExpression.java @@ -496,16 +496,31 @@ class JunctionExpression implements SpiJunction, SpiExpression, Expression return exprList.forUpdate(); } + @Override + public Query forUpdate(Query.LockType lockType) { + return exprList.forUpdate(lockType); + } + @Override public Query forUpdateNoWait() { return exprList.forUpdateNoWait(); } + @Override + public Query forUpdateNoWait(Query.LockType lockType) { + return exprList.forUpdateNoWait(lockType); + } + @Override public Query forUpdateSkipLocked() { return exprList.forUpdateSkipLocked(); } + @Override + public Query forUpdateSkipLocked(Query.LockType lockType) { + return exprList.forUpdateSkipLocked(lockType); + } + /** * Path exists - for the given path in a JSON document. */ diff --git a/ebean-core/src/test/java/io/ebean/config/ServerConfigTest.java b/ebean-core/src/test/java/io/ebean/config/ServerConfigTest.java index 56c62705f..a81082d16 100644 --- a/ebean-core/src/test/java/io/ebean/config/ServerConfigTest.java +++ b/ebean-core/src/test/java/io/ebean/config/ServerConfigTest.java @@ -73,7 +73,7 @@ public class ServerConfigTest { props.setProperty("caseSensitiveCollation", "false"); props.setProperty("loadModuleInfo", "true"); props.setProperty("collectQueryPlanThresholdMicros", "10000"); - props.setProperty("lockWithKey", "false"); + props.setProperty("forUpdateNoKey", "true"); serverConfig.loadFromProperties(props); @@ -85,7 +85,7 @@ public class ServerConfigTest { assertTrue(serverConfig.isIdGeneratorAutomatic()); assertFalse(serverConfig.getPlatformConfig().isCaseSensitiveCollation()); - assertFalse(serverConfig.getPlatformConfig().isLockWithKey()); + assertTrue(serverConfig.getPlatformConfig().isForUpdateNoKey()); assertThat(serverConfig.getNamingConvention()).isInstanceOf(MatchingNamingConvention.class); diff --git a/ebean-core/src/test/java/io/ebean/config/dbplatform/PostgresPlatformTest.java b/ebean-core/src/test/java/io/ebean/config/dbplatform/PostgresPlatformTest.java index a48dc2772..6a26330db 100644 --- a/ebean-core/src/test/java/io/ebean/config/dbplatform/PostgresPlatformTest.java +++ b/ebean-core/src/test/java/io/ebean/config/dbplatform/PostgresPlatformTest.java @@ -29,7 +29,7 @@ public class PostgresPlatformTest { PlatformConfig config = new PlatformConfig(); platform.configure(config); - assertThat(config.isLockWithKey()).isTrue(); + assertThat(config.isForUpdateNoKey()).isFalse(); assertThat(platform.withForUpdate("X", Query.ForUpdate.SKIPLOCKED, Query.LockType.Default)).isEqualTo("X for update skip locked"); assertThat(platform.withForUpdate("X", Query.ForUpdate.NOWAIT, Query.LockType.Default)).isEqualTo("X for update nowait"); assertThat(platform.withForUpdate("X", Query.ForUpdate.BASE, Query.LockType.Default)).isEqualTo("X for update"); @@ -56,10 +56,10 @@ public class PostgresPlatformTest { DatabasePlatform platform = new PostgresPlatform(); PlatformConfig config = new PlatformConfig(); - config.setLockWithKey(false); + config.setForUpdateNoKey(true); platform.configure(config); - assertThat(config.isLockWithKey()).isFalse(); + assertThat(config.isForUpdateNoKey()).isTrue(); assertThat(platform.withForUpdate("X", Query.ForUpdate.SKIPLOCKED, Query.LockType.Default)).isEqualTo("X for no key update skip locked"); assertThat(platform.withForUpdate("X", Query.ForUpdate.NOWAIT, Query.LockType.Default)).isEqualTo("X for no key update nowait"); assertThat(platform.withForUpdate("X", Query.ForUpdate.BASE, Query.LockType.Default)).isEqualTo("X for no key update");