From 9c435f3ff82a6d61c0d3bc6232b3d2a8bba758d9 Mon Sep 17 00:00:00 2001 From: Lukas Eder Date: Mon, 30 Apr 2012 23:55:29 +0200 Subject: [PATCH] [#1296] Simulate the FOR UPDATE clause for SQL Server, CUBRID, using JDBC's ResultSet.CONCUR_UPDATABLE --- jOOQ-test/src/org/jooq/test/BaseTest.java | 11 ++ .../jooq/test/_/testcases/SelectTests.java | 111 ++++++++++++++---- .../src/org/jooq/test/jOOQAbstractTest.java | 13 +- jOOQ/src/main/java/org/jooq/LockProvider.java | 27 ++++- .../java/org/jooq/SelectForUpdateStep.java | 4 +- .../org/jooq/impl/AbstractResultQuery.java | 22 +++- .../java/org/jooq/impl/AbstractSubSelect.java | 10 +- .../main/java/org/jooq/impl/CursorImpl.java | 8 ++ .../java/org/jooq/impl/SQLResultQuery.java | 5 + jOOQ/src/main/java/org/jooq/impl/Union.java | 5 + 10 files changed, 180 insertions(+), 36 deletions(-) diff --git a/jOOQ-test/src/org/jooq/test/BaseTest.java b/jOOQ-test/src/org/jooq/test/BaseTest.java index 4f0b742da1..741fff30d0 100644 --- a/jOOQ-test/src/org/jooq/test/BaseTest.java +++ b/jOOQ-test/src/org/jooq/test/BaseTest.java @@ -659,6 +659,10 @@ public abstract class BaseTest< return delegate.getConnection(); } + protected final Connection getNewConnection() { + return delegate.getConnection0(null, null); + } + protected final Factory create() { return delegate.create(); } @@ -671,6 +675,13 @@ public abstract class BaseTest< return delegate.internal(q); } + protected final void sleep(long millis) { + try { + Thread.sleep(millis); + } + catch (InterruptedException ignore) {} + } + @SuppressWarnings("unchecked") protected Sequence SAuthorID() throws IllegalAccessException, NoSuchFieldException { return (Sequence) cSequences().getField("S_AUTHOR_ID").get(cSequences()); diff --git a/jOOQ-test/src/org/jooq/test/_/testcases/SelectTests.java b/jOOQ-test/src/org/jooq/test/_/testcases/SelectTests.java index 40c2ff6108..ba3feab1f4 100644 --- a/jOOQ-test/src/org/jooq/test/_/testcases/SelectTests.java +++ b/jOOQ-test/src/org/jooq/test/_/testcases/SelectTests.java @@ -35,6 +35,7 @@ */ package org.jooq.test._.testcases; +import static java.util.Arrays.asList; import static junit.framework.Assert.assertEquals; import static junit.framework.Assert.assertTrue; import static org.jooq.impl.Factory.count; @@ -42,6 +43,8 @@ import static org.jooq.impl.Factory.countDistinct; import static org.jooq.impl.Factory.trim; import static org.jooq.impl.Factory.val; +import java.util.Vector; + import org.jooq.Field; import org.jooq.Record; import org.jooq.Result; @@ -50,6 +53,8 @@ import org.jooq.SelectQuery; import org.jooq.Table; import org.jooq.TableRecord; import org.jooq.UpdatableRecord; +import org.jooq.exception.DataAccessException; +import org.jooq.impl.Factory; import org.jooq.test.BaseTest; import org.jooq.test.jOOQAbstractTest; @@ -342,25 +347,91 @@ extends BaseTest result = create().select(TAuthor_ID()) - .from(TAuthor()) - .forUpdate() - .fetch(); - assertEquals(2, result.size()); - Result result2 = create().selectFrom(TAuthor()) - .forUpdate() - .fetch(); - assertEquals(2, result2.size()); + // Checking for syntax correctness and locking behaviour + // ----------------------------------------------------- + final Factory create1 = create(); + final Factory create2 = create(); + + create2.setConnection(getNewConnection()); + create2.getConnection().setAutoCommit(false); + + final Vector execOrder = new Vector(); + + try { + final Thread t1 = new Thread(new Runnable() { + @Override + public void run() { + sleep(2000); + execOrder.add("t1-block"); + try { + create1 + .select(TAuthor_ID()) + .from(TAuthor()) + .forUpdate() + .fetch(); + } + + // Some databases fail on locking, others lock for a while + catch (DataAccessException ignore) { + } + finally { + execOrder.add("t1-fail-or-t2-commit"); + } + } + }); + + final Thread t2 = new Thread(new Runnable() { + @Override + public void run() { + execOrder.add("t2-exec"); + Result result2 = create2 + .selectFrom(TAuthor()) + .forUpdate() + .fetch(); + assertEquals(2, result2.size()); + + execOrder.add("t2-signal"); + sleep(4000); + execOrder.add("t1-fail-or-t2-commit"); + + try { + create2.getConnection().commit(); + create2.getConnection().close(); + } + catch (Exception e) {} + } + }); + + // This is the test case: + // 0.0s: Both threads start + // 0.0s: t1 sleeps for 2s + // 0.0s: t2 locks the T_AUTHOR table + // 0.1s: t2 sleeps for 4s + // 2.0s: t1 blocks on the T_AUTHOR table + // ???s: t1 fails + // 4.0s: t2 commits and unlocks T_AUTHOR + t1.start(); + t2.start(); + + t1.join(); + t2.join(); + + assertEquals(asList("t2-exec", "t2-signal", "t1-block", "t1-fail-or-t2-commit", "t1-fail-or-t2-commit"), execOrder); + } + finally { + try { + create2.getConnection().close(); + } + catch (Exception e) {} + } // Check again with limit / offset clauses + // --------------------------------------- switch (getDialect()) { case INGRES: case ORACLE: @@ -398,7 +469,7 @@ extends BaseTest result = create().select(TAuthor_ID()) .from(TAuthor()) .forUpdate() .wait(2) @@ -418,7 +489,7 @@ extends BaseTest result2 = create().selectFrom(TAuthor()) .forUpdate() .of(TAuthor_LAST_NAME(), TAuthor_FIRST_NAME()) .wait(2) @@ -452,14 +523,14 @@ extends BaseTest result = create().select(TAuthor_ID()) .from(TAuthor()) .forUpdate() .of(TAuthor_LAST_NAME(), TAuthor_FIRST_NAME()) .fetch(); assertEquals(2, result.size()); - result2 = create().selectFrom(TAuthor()) + Result result2 = create().selectFrom(TAuthor()) .forUpdate() .of(TAuthor_LAST_NAME(), TAuthor_FIRST_NAME()) .fetch(); @@ -470,14 +541,14 @@ extends BaseTest result = create().select(TAuthor_ID()) .from(TAuthor()) .forUpdate() .of(TAuthor()) .fetch(); assertEquals(2, result.size()); - result2 = create().selectFrom(TAuthor()) + Result result2 = create().selectFrom(TAuthor()) .forUpdate() .of(TAuthor()) .fetch(); @@ -491,13 +562,13 @@ extends BaseTest result = create().select(TAuthor_ID()) .from(TAuthor()) .forShare() .fetch(); assertEquals(2, result.size()); - result2 = create().selectFrom(TAuthor()) + Result result2 = create().selectFrom(TAuthor()) .forShare() .fetch(); assertEquals(2, result2.size()); diff --git a/jOOQ-test/src/org/jooq/test/jOOQAbstractTest.java b/jOOQ-test/src/org/jooq/test/jOOQAbstractTest.java index beafdfb04b..5a6ea69339 100644 --- a/jOOQ-test/src/org/jooq/test/jOOQAbstractTest.java +++ b/jOOQ-test/src/org/jooq/test/jOOQAbstractTest.java @@ -54,8 +54,6 @@ import java.util.List; import java.util.Map; import java.util.Properties; -import javax.swing.UIManager; - import org.jooq.ArrayRecord; import org.jooq.DataType; import org.jooq.ExecuteType; @@ -76,7 +74,6 @@ import org.jooq.conf.RenderMapping; import org.jooq.conf.Settings; import org.jooq.conf.SettingsTools; import org.jooq.debug.DebugListener; -import org.jooq.debug.console.Console; import org.jooq.debug.console.DatabaseDescriptor; import org.jooq.debug.console.remote.RemoteDebuggerServer; import org.jooq.impl.Factory; @@ -441,10 +438,10 @@ public abstract class jOOQAbstractTest< }; try { - UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName()); - Console console = new Console(descriptor, true); - console.setLoggingActive(true); - console.setVisible(true); +// UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName()); +// Console console = new Console(descriptor, true); +// console.setLoggingActive(true); +// console.setVisible(true); } catch (Exception ignore) {} } @@ -471,7 +468,7 @@ public abstract class jOOQAbstractTest< return connectionMultiSchemaUnused; } - private final Connection getConnection0(String jdbcUser, String jdbcPassword) { + final Connection getConnection0(String jdbcUser, String jdbcPassword) { try { String configuration = System.getProperty("jdbc.properties"); if (configuration == null) { diff --git a/jOOQ/src/main/java/org/jooq/LockProvider.java b/jOOQ/src/main/java/org/jooq/LockProvider.java index 7fe662f696..a9a120d373 100644 --- a/jOOQ/src/main/java/org/jooq/LockProvider.java +++ b/jOOQ/src/main/java/org/jooq/LockProvider.java @@ -36,6 +36,7 @@ package org.jooq; import static org.jooq.SQLDialect.ASE; +import static org.jooq.SQLDialect.CUBRID; import static org.jooq.SQLDialect.DB2; import static org.jooq.SQLDialect.DERBY; import static org.jooq.SQLDialect.H2; @@ -44,8 +45,12 @@ import static org.jooq.SQLDialect.INGRES; import static org.jooq.SQLDialect.MYSQL; import static org.jooq.SQLDialect.ORACLE; import static org.jooq.SQLDialect.POSTGRES; +import static org.jooq.SQLDialect.SQLSERVER; import static org.jooq.SQLDialect.SYBASE; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; import java.util.Collection; /** @@ -59,7 +64,8 @@ public interface LockProvider { /** * Sets the "FOR UPDATE" flag onto the query *

- * This has been observed to be supported by any of these dialects: + *

Native implementation

This has been observed to be supported by + * any of these dialects: * + *

Simulation

These dialects can simulate the + * FOR UPDATE clause using a cursor. The cursor is handled by + * the JDBC driver, at {@link PreparedStatement} construction time, when + * calling {@link Connection#prepareStatement(String, int, int)} with + * {@link ResultSet#CONCUR_UPDATABLE}. jOOQ handles simulation of a + * FOR UPDATE clause using CONCUR_UPDATABLE for + * these dialects: + * *

- * These dialects are known not to support the FOR UPDATE - * clause in regular SQL: + * Note: This simulation may not be efficient for large result sets! + *

Not supported

These dialects are known not to support the + * FOR UPDATE clause in regular SQL: * *

* If your dialect does not support this clause, jOOQ will still render it, @@ -98,7 +115,7 @@ public interface LockProvider { * * @param forUpdate The flag's value */ - @Support({ ASE, DB2, DERBY, H2, HSQLDB, INGRES, MYSQL, ORACLE, POSTGRES, SYBASE }) + @Support({ ASE, CUBRID, DB2, DERBY, H2, HSQLDB, INGRES, MYSQL, ORACLE, POSTGRES, SQLSERVER, SYBASE }) void setForUpdate(boolean forUpdate); /** diff --git a/jOOQ/src/main/java/org/jooq/SelectForUpdateStep.java b/jOOQ/src/main/java/org/jooq/SelectForUpdateStep.java index f4331c8480..a814f25c51 100644 --- a/jOOQ/src/main/java/org/jooq/SelectForUpdateStep.java +++ b/jOOQ/src/main/java/org/jooq/SelectForUpdateStep.java @@ -36,6 +36,7 @@ package org.jooq; import static org.jooq.SQLDialect.ASE; +import static org.jooq.SQLDialect.CUBRID; import static org.jooq.SQLDialect.DB2; import static org.jooq.SQLDialect.DERBY; import static org.jooq.SQLDialect.H2; @@ -44,6 +45,7 @@ import static org.jooq.SQLDialect.INGRES; import static org.jooq.SQLDialect.MYSQL; import static org.jooq.SQLDialect.ORACLE; import static org.jooq.SQLDialect.POSTGRES; +import static org.jooq.SQLDialect.SQLSERVER; import static org.jooq.SQLDialect.SYBASE; /** @@ -101,7 +103,7 @@ public interface SelectForUpdateStep extends SelectFinalStep { * * @see LockProvider#setForUpdate(boolean) see LockProvider for more details */ - @Support({ ASE, DB2, DERBY, H2, HSQLDB, INGRES, MYSQL, ORACLE, POSTGRES, SYBASE }) + @Support({ ASE, CUBRID, DB2, DERBY, H2, HSQLDB, INGRES, MYSQL, ORACLE, POSTGRES, SQLSERVER, SYBASE }) SelectForUpdateOfStep forUpdate(); /** diff --git a/jOOQ/src/main/java/org/jooq/impl/AbstractResultQuery.java b/jOOQ/src/main/java/org/jooq/impl/AbstractResultQuery.java index d5cb450051..a729418619 100644 --- a/jOOQ/src/main/java/org/jooq/impl/AbstractResultQuery.java +++ b/jOOQ/src/main/java/org/jooq/impl/AbstractResultQuery.java @@ -35,8 +35,13 @@ */ package org.jooq.impl; +import static java.sql.ResultSet.CONCUR_UPDATABLE; +import static java.sql.ResultSet.TYPE_SCROLL_SENSITIVE; +import static java.util.Arrays.asList; import static java.util.concurrent.Executors.newSingleThreadExecutor; import static org.jooq.SQLDialect.ASE; +import static org.jooq.SQLDialect.CUBRID; +import static org.jooq.SQLDialect.SQLSERVER; import java.lang.reflect.Array; import java.sql.Connection; @@ -114,7 +119,17 @@ abstract class AbstractResultQuery extends AbstractQuery imple @Override protected final void prepare(ExecuteContext ctx) throws SQLException { - super.prepare(ctx); + + // [#1296] These dialects do not implement FOR UPDATE. But the same + // effect can be achieved using ResultSet.CONCUR_UPDATABLE + if (isForUpdate() && asList(CUBRID, SQLSERVER).contains(ctx.getDialect())) { + ctx.statement(ctx.getConnection().prepareStatement(ctx.sql(), TYPE_SCROLL_SENSITIVE, CONCUR_UPDATABLE)); + } + + // Regular behaviour + else { + ctx.statement(ctx.getConnection().prepareStatement(ctx.sql())); + } // [#1263] Allow for negative fetch sizes to support some non-standard // MySQL feature, where Integer.MIN_VALUE is used @@ -228,6 +243,11 @@ abstract class AbstractResultQuery extends AbstractQuery imple */ abstract boolean isSelectingRefCursor(); + /** + * Subclasses should indicate whether they want an updatable {@link ResultSet} + */ + abstract boolean isForUpdate(); + @Override public final Result fetch() { execute(); diff --git a/jOOQ/src/main/java/org/jooq/impl/AbstractSubSelect.java b/jOOQ/src/main/java/org/jooq/impl/AbstractSubSelect.java index 96ccf3b35c..6f9e1b2a41 100644 --- a/jOOQ/src/main/java/org/jooq/impl/AbstractSubSelect.java +++ b/jOOQ/src/main/java/org/jooq/impl/AbstractSubSelect.java @@ -35,6 +35,8 @@ */ package org.jooq.impl; +import static java.util.Arrays.asList; +import static org.jooq.SQLDialect.CUBRID; import static org.jooq.SQLDialect.SQLSERVER; import static org.jooq.impl.Factory.literal; import static org.jooq.impl.Factory.one; @@ -248,7 +250,8 @@ implements toSQLReference0(context); } - if (forUpdate) { + // [#1296] FOR UPDATE is simulated in some dialects using ResultSet.CONCUR_UPDATABLE + if (forUpdate && !asList(CUBRID, SQLSERVER).contains(context.getDialect())) { context.formatSeparator() .keyword("for update"); @@ -815,6 +818,11 @@ implements this.hint = hint; } + @Override + final boolean isForUpdate() { + return forUpdate; + } + // ------------------------------------------------------------------------- // Utility classes // ------------------------------------------------------------------------- diff --git a/jOOQ/src/main/java/org/jooq/impl/CursorImpl.java b/jOOQ/src/main/java/org/jooq/impl/CursorImpl.java index 973cf44000..6ab870f25e 100644 --- a/jOOQ/src/main/java/org/jooq/impl/CursorImpl.java +++ b/jOOQ/src/main/java/org/jooq/impl/CursorImpl.java @@ -1242,6 +1242,14 @@ class CursorImpl implements Cursor { try { if (!isClosed && rs.next()) { + + // [#1296] Force a row-lock by updating the row if the + // FOR UPDATE clause is simulated + if (rs.getConcurrency() == ResultSet.CONCUR_UPDATABLE) { + rs.updateObject(1, rs.getObject(1)); + rs.updateRow(); + } + record = Util.newRecord(type, fields, ctx.configuration()); ctx.record(record); diff --git a/jOOQ/src/main/java/org/jooq/impl/SQLResultQuery.java b/jOOQ/src/main/java/org/jooq/impl/SQLResultQuery.java index d3c35583b3..b6bcc7a746 100644 --- a/jOOQ/src/main/java/org/jooq/impl/SQLResultQuery.java +++ b/jOOQ/src/main/java/org/jooq/impl/SQLResultQuery.java @@ -104,6 +104,11 @@ class SQLResultQuery extends AbstractResultQuery implements BindingProvi return false; } + @Override + final boolean isForUpdate() { + return false; + } + // ------------------------------------------------------------------------ // QueryPart API // ------------------------------------------------------------------------ diff --git a/jOOQ/src/main/java/org/jooq/impl/Union.java b/jOOQ/src/main/java/org/jooq/impl/Union.java index b81eb2e7e3..2c6401eec6 100644 --- a/jOOQ/src/main/java/org/jooq/impl/Union.java +++ b/jOOQ/src/main/java/org/jooq/impl/Union.java @@ -129,4 +129,9 @@ class Union extends AbstractSelect { public final void bind(BindContext context) { context.bind(queries); } + + @Override + final boolean isForUpdate() { + return false; + } }