HHH-10654 - LockOptions.SKIP_LOCKED semantics implementation on MSSQL

hibernate-core/hibernate-core.gradle

@@ -258,4 +258,7 @@ test.dependsOn ":hibernate-orm-modules:prepareWildFlyForTests"
 
 test {
     systemProperty "file.encoding", "utf-8"
+    beforeTest { descriptor ->
+        //println "Starting test: " + descriptor
+    }
 }

hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java

@@ -2882,6 +2882,15 @@ public boolean supportsValuesList() {
 		return false;
 	}
 
+	/**
+	 * Does this dialect/database support SKIP_LOCKED timeout.
+	 *
+	 * @return {@code true} if SKIP_LOCKED is supported
+	 */
+	public boolean supportsSkipLocked() {
+		return false;
+	}
+
 	public boolean isLegacyLimitHandlerBehaviorEnabled() {
 		return legacyLimitHandlerBehavior;
 	}

hibernate-core/src/main/java/org/hibernate/dialect/Oracle10gDialect.java

@@ -85,4 +85,9 @@ public int registerResultSetOutParameter(CallableStatement statement, String nam
 	public ResultSet getResultSet(CallableStatement statement, String name) throws SQLException {
 		return (ResultSet) statement.getObject( name );
 	}
+
+	@Override
+	public boolean supportsSkipLocked() {
+		return true;
+	}
 }

hibernate-core/src/main/java/org/hibernate/dialect/PostgreSQL95Dialect.java

@@ -62,4 +62,9 @@ public String getForUpdateSkipLockedString() {
 	public String getForUpdateSkipLockedString(String aliases) {
 		return getForUpdateString() + " of " + aliases + " skip locked";
 	}
+
+	@Override
+	public boolean supportsSkipLocked() {
+		return true;
+	}
 }

hibernate-core/src/main/java/org/hibernate/dialect/SQLServer2005Dialect.java

@@ -64,24 +64,31 @@ protected LimitHandler getDefaultLimitHandler() {
 
 	@Override
 	public String appendLockHint(LockOptions lockOptions, String tableName) {
-		// NOTE : since SQLServer2005 the nowait hint is supported
-		if ( lockOptions.getLockMode() == LockMode.UPGRADE_NOWAIT ) {
-			return tableName + " with (updlock, rowlock, nowait)";
+
+		LockMode lockMode = lockOptions.getAliasSpecificLockMode( tableName );
+		if(lockMode == null) {
+			lockMode = lockOptions.getLockMode();
 		}
 
-		final LockMode mode = lockOptions.getLockMode();
-		final boolean isNoWait = lockOptions.getTimeOut() == LockOptions.NO_WAIT;
-		final String noWaitStr = isNoWait ? ", nowait" : "";
-		switch ( mode ) {
+		final String writeLockStr = lockOptions.getTimeOut() == LockOptions.SKIP_LOCKED ? "updlock" : "updlock, holdlock";
+		final String readLockStr = lockOptions.getTimeOut() == LockOptions.SKIP_LOCKED ? "updlock" : "holdlock";
+
+		final String noWaitStr = lockOptions.getTimeOut() == LockOptions.NO_WAIT ? ", nowait" : "";
+		final String skipLockStr = lockOptions.getTimeOut() == LockOptions.SKIP_LOCKED ? ", readpast" : "";
+
+		switch ( lockMode ) {
 			case UPGRADE:
 			case PESSIMISTIC_WRITE:
 			case WRITE: {
-				return tableName + " with (updlock, rowlock" + noWaitStr + ")";
+				return tableName + " with (" + writeLockStr + ", rowlock" + noWaitStr + skipLockStr + ")";
 			}
 			case PESSIMISTIC_READ: {
-				return tableName + " with (holdlock, rowlock" + noWaitStr + ")";
-			}case UPGRADE_SKIPLOCKED:
+				return tableName + " with (" + readLockStr + ", rowlock" + noWaitStr + skipLockStr + ")";
+			}
+			case UPGRADE_SKIPLOCKED:
 				return tableName + " with (updlock, rowlock, readpast" + noWaitStr + ")";
+			case UPGRADE_NOWAIT:
+				return tableName + " with (updlock, holdlock, rowlock, nowait)";
 			default: {
 				return tableName;
 			}
@@ -110,4 +117,9 @@ public JDBCException convert(SQLException sqlException, String message, String s
 	public boolean supportsNonQueryWithCTE() {
 		return true;
 	}
+
+	@Override
+	public boolean supportsSkipLocked() {
+		return true;
+	}
 }

hibernate-core/src/test/java/org/hibernate/dialect/SQLServer2005DialectTestCase.java

@@ -446,7 +446,7 @@ public void testAppendLockHintPessimisticReadNoTimeOut() {
 	@Test
 	@TestForIssue(jiraKey = "HHH-9635")
 	public void testAppendLockHintWrite() {
-		final String expectedLockHint = "tab1 with (updlock, rowlock)";
+		final String expectedLockHint = "tab1 with (updlock, holdlock, rowlock)";
 
 		LockOptions lockOptions = new LockOptions( LockMode.WRITE );
 		String lockHint = dialect.appendLockHint( lockOptions, "tab1" );
@@ -457,7 +457,7 @@ public void testAppendLockHintWrite() {
 	@Test
 	@TestForIssue(jiraKey = "HHH-9635")
 	public void testAppendLockHintWriteWithNoTimeOut() {
-		final String expectedLockHint = "tab1 with (updlock, rowlock, nowait)";
+		final String expectedLockHint = "tab1 with (updlock, holdlock, rowlock, nowait)";
 
 		LockOptions lockOptions = new LockOptions( LockMode.WRITE );
 		lockOptions.setTimeOut( LockOptions.NO_WAIT );
@@ -470,7 +470,7 @@ public void testAppendLockHintWriteWithNoTimeOut() {
 	@Test
 	@TestForIssue(jiraKey = "HHH-9635")
 	public void testAppendLockHintUpgradeNoWait() {
-		final String expectedLockHint = "tab1 with (updlock, rowlock, nowait)";
+		final String expectedLockHint = "tab1 with (updlock, holdlock, rowlock, nowait)";
 
 		LockOptions lockOptions = new LockOptions( LockMode.UPGRADE_NOWAIT );
 		String lockHint = dialect.appendLockHint( lockOptions, "tab1" );
@@ -481,7 +481,7 @@ public void testAppendLockHintUpgradeNoWait() {
 	@Test
 	@TestForIssue(jiraKey = "HHH-9635")
 	public void testAppendLockHintUpgradeNoWaitNoTimeout() {
-		final String expectedLockHint = "tab1 with (updlock, rowlock, nowait)";
+		final String expectedLockHint = "tab1 with (updlock, holdlock, rowlock, nowait)";
 
 		LockOptions lockOptions = new LockOptions( LockMode.UPGRADE_NOWAIT );
 		lockOptions.setTimeOut( LockOptions.NO_WAIT );
@@ -493,7 +493,7 @@ public void testAppendLockHintUpgradeNoWaitNoTimeout() {
 	@Test
 	@TestForIssue(jiraKey = "HHH-9635")
 	public void testAppendLockHintUpgrade() {
-		final String expectedLockHint = "tab1 with (updlock, rowlock)";
+		final String expectedLockHint = "tab1 with (updlock, holdlock, rowlock)";
 
 		LockOptions lockOptions = new LockOptions( LockMode.UPGRADE );
 		String lockHint = dialect.appendLockHint( lockOptions, "tab1" );
@@ -504,7 +504,7 @@ public void testAppendLockHintUpgrade() {
 	@Test
 	@TestForIssue(jiraKey = "HHH-9635")
 	public void testAppendLockHintUpgradeNoTimeout() {
-		final String expectedLockHint = "tab1 with (updlock, rowlock, nowait)";
+		final String expectedLockHint = "tab1 with (updlock, holdlock, rowlock, nowait)";
 
 		LockOptions lockOptions = new LockOptions( LockMode.UPGRADE );
 		lockOptions.setTimeOut( LockOptions.NO_WAIT );
@@ -516,7 +516,7 @@ public void testAppendLockHintUpgradeNoTimeout() {
 	@Test
 	@TestForIssue(jiraKey = "HHH-9635")
 	public void testAppendLockHintPessimisticWrite() {
-		final String expectedLockHint = "tab1 with (updlock, rowlock)";
+		final String expectedLockHint = "tab1 with (updlock, holdlock, rowlock)";
 
 		LockOptions lockOptions = new LockOptions( LockMode.UPGRADE );
 		String lockHint = dialect.appendLockHint( lockOptions, "tab1" );
@@ -527,7 +527,7 @@ public void testAppendLockHintPessimisticWrite() {
 	@Test
 	@TestForIssue(jiraKey = "HHH-9635")
 	public void testAppendLockHintPessimisticWriteNoTimeOut() {
-		final String expectedLockHint = "tab1 with (updlock, rowlock, nowait)";
+		final String expectedLockHint = "tab1 with (updlock, holdlock, rowlock, nowait)";
 
 		LockOptions lockOptions = new LockOptions( LockMode.UPGRADE );
 		lockOptions.setTimeOut( LockOptions.NO_WAIT );

hibernate-core/src/test/java/org/hibernate/jpa/test/lock/LockTest.java

@@ -13,6 +13,7 @@
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.FutureTask;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
 import javax.persistence.EntityManager;
 import javax.persistence.LockModeType;
 import javax.persistence.LockTimeoutException;
@@ -22,9 +23,12 @@
 import javax.persistence.Query;
 import javax.persistence.QueryTimeoutException;
 
+import org.hibernate.Session;
+import org.hibernate.dialect.Dialect;
 import org.hibernate.dialect.HSQLDialect;
 import org.hibernate.dialect.Oracle10gDialect;
 import org.hibernate.dialect.PostgreSQL81Dialect;
+import org.hibernate.dialect.SQLServerDialect;
 import org.hibernate.dialect.SybaseASE15Dialect;
 import org.hibernate.jpa.AvailableSettings;
 import org.hibernate.jpa.QueryHints;
@@ -40,6 +44,7 @@
 
 import org.jboss.logging.Logger;
 
+import static org.hibernate.testing.transaction.TransactionUtil.doInJPA;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
@@ -108,6 +113,7 @@ public void testFindWithPessimisticWriteLockTimeoutException() {
 		}
 		catch (LockTimeoutException lte) {
 			// Proper exception thrown for dialect supporting lock timeouts when an immediate timeout is set.
+			lte.getCause();
 		}
 		catch (PessimisticLockException pe) {
 			fail( "Find with immediate timeout should have thrown LockTimeoutException." );
@@ -134,6 +140,100 @@ public void testFindWithPessimisticWriteLockTimeoutException() {
 		em2.getTransaction().commit();
 		em2.close();
 	}
+
+	@Test
+	@RequiresDialectFeature( value = DialectChecks.SupportSkipLocked.class )
+	public void testUpdateWithPessimisticReadLockSkipLocked() {
+		Lock lock = new Lock();
+		lock.setName( "name" );
+
+		doInJPA( this::entityManagerFactory, entityManager -> {
+			entityManager.persist( lock );
+		} );
+
+		EntityManager em2 = createIsolatedEntityManager();
+		em2.getTransaction().begin();
+
+		try {
+			Map<String, Object> properties = new HashMap<>();
+			properties.put( org.hibernate.cfg.AvailableSettings.JPA_LOCK_TIMEOUT, -2L );
+			em2.find( Lock.class, lock.getId(), LockModeType.PESSIMISTIC_READ, properties );
+
+			try {
+				doInJPA( this::entityManagerFactory, entityManager -> {
+					entityManager.createNativeQuery( updateStatement() )
+							.setParameter( "name", "changed" )
+							.setParameter( "id", lock.getId() )
+							.executeUpdate();
+				} );
+				fail("Should throw LockTimeoutException");
+			}
+			catch (LockTimeoutException expected) {
+			}
+		}
+		finally {
+			em2.getTransaction().commit();
+		}
+
+		doInJPA( this::entityManagerFactory, entityManager -> {
+			Lock _lock = entityManager.merge( lock );
+			entityManager.remove( _lock );
+		} );
+	}
+
+	@Test
+	@RequiresDialectFeature(value = DialectChecks.SupportsLockTimeouts.class)
+	public void testUpdateWithPessimisticReadLockWithoutNoWait() {
+		Lock lock = new Lock();
+		lock.setName( "name" );
+
+		doInJPA( this::entityManagerFactory, entityManager -> {
+			entityManager.persist( lock );
+		} );
+
+		EntityManager em2 = createIsolatedEntityManager();
+		em2.getTransaction().begin();
+
+		try {
+			em2.find( Lock.class, lock.getId(), LockModeType.PESSIMISTIC_READ );
+
+			AtomicBoolean failureExpected = new AtomicBoolean();
+
+			try {
+				doInJPA( this::entityManagerFactory, entityManager -> {
+					try {
+						entityManager.createNativeQuery( updateStatement() )
+								.setParameter( "name", "changed" )
+								.setParameter( "id", lock.getId() )
+								.executeUpdate();
+					}
+					catch (LockTimeoutException | PessimisticLockException expected) {
+						failureExpected.set( true );
+					}
+				} );
+			}
+			catch (Exception e) {
+				if ( !failureExpected.get() ) {
+					fail( "Should throw LockTimeoutException or PessimisticLockException" );
+				}
+			}
+		}
+		finally {
+			em2.getTransaction().commit();
+		}
+
+		doInJPA( this::entityManagerFactory, entityManager -> {
+			Lock _lock = entityManager.merge( lock );
+			entityManager.remove( _lock );
+		} );
+	}
+
+	protected String updateStatement() {
+		if( SQLServerDialect.class.isAssignableFrom( Dialect.getDialect().getClass() ) ) {
+			return "UPDATE Lock_ WITH(NOWAIT) SET name = :name where id = :id";
+		}
+		return "UPDATE Lock_ SET name = :name where id = :id";
+	}
 
 	@Test
 	public void testLockRead() throws Exception {

hibernate-core/src/test/java/org/hibernate/test/dialect/unit/lockhint/SQLServer2005LockHintsTest.java

@@ -18,7 +18,7 @@ public class SQLServer2005LockHintsTest extends AbstractLockHintTest {
 	public static final Dialect DIALECT = new SQLServer2005Dialect();
 
 	protected String getLockHintUsed() {
-		return "with (updlock, rowlock, nowait)";
+		return "with (updlock, holdlock, rowlock, nowait)";
 	}
 
 	protected Dialect getDialectUnderTest() {

hibernate-core/src/test/java/org/hibernate/test/locking/A.java

@@ -41,7 +41,7 @@ public void setId(Long id) {
 		this.id = id;
 	}
 
-	@Column(name="`value`")
+	@Column(name="a_value")
 	public String getValue() {
 		return value;
 	}