HHH-12866 : apply "for update of" with aliases in Oracle dialect

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

@@ -10,11 +10,17 @@
 import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.sql.Types;
+import java.util.HashSet;
+import java.util.Iterator;
 import java.util.Locale;
+import java.util.Map;
+import java.util.Set;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
 import org.hibernate.JDBCException;
+import org.hibernate.LockMode;
+import org.hibernate.LockOptions;
 import org.hibernate.QueryTimeoutException;
 import org.hibernate.cfg.Environment;
 import org.hibernate.dialect.function.NoArgSQLFunction;
@@ -38,6 +44,7 @@
 import org.hibernate.hql.spi.id.global.GlobalTemporaryTableBulkIdStrategy;
 import org.hibernate.hql.spi.id.local.AfterUseAction;
 import org.hibernate.internal.util.JdbcExceptionHelper;
+import org.hibernate.internal.util.StringHelper;
 import org.hibernate.procedure.internal.StandardCallableStatementSupport;
 import org.hibernate.procedure.spi.CallableStatementSupport;
 import org.hibernate.sql.CaseFragment;
@@ -465,12 +472,140 @@ public boolean supportsLimit() {
 
 	@Override
 	public String getForUpdateString(String aliases) {
-		return getForUpdateString() + " of " + aliases;
+		StringBuilder sb = new StringBuilder();
+		sb.append( getForUpdateString() );
+		if ( StringHelper.isNotEmpty( aliases ) ) {
+			sb.append( " of " ).append( aliases );
+		}
+		return sb.toString();
 	}
 
 	@Override
 	public String getForUpdateNowaitString(String aliases) {
-		return getForUpdateString() + " of " + aliases + " nowait";
+		return getForUpdateString( aliases ) + " nowait";
+	}
+
+	/*
+	 * Overwrite because the parent's implementation does not support the `for update of ...` syntax.
+	 *
+	 * Since Oracle 8i (or even prior versions) the syntax of "for update of [table.column]" is already supported.
+	 * Refer to https://docs.oracle.com/cd/A87860_01/doc/server.817/a85397/state21b.htm#2065648
+	 */
+	@Override
+	public String getForUpdateString(String aliases, LockOptions lockOptions) {
+		LockMode lockMode = lockOptions.getLockMode();
+		final Iterator<Map.Entry<String, LockMode>> itr = lockOptions.getAliasLockIterator();
+		Set<String> tableAliasSet = new HashSet<>();
+		while ( itr.hasNext() ) {
+			// seek the highest lock mode
+			final Map.Entry<String, LockMode> entry = itr.next();
+			tableAliasSet.add( entry.getKey() );
+			final LockMode lm = entry.getValue();
+			if ( lm.greaterThan( lockMode ) ) {
+				lockMode = lm;
+			}
+		}
+		lockOptions.setLockMode( lockMode );
+		if ( needToSpecifyAliasesInForUpdate( tableAliasSet, aliases ) ) {
+			return getForUpdateString( lockMode, lockOptions.getTimeOut(), aliases );
+		}
+		else {
+			return getForUpdateString( lockOptions );
+		}
+	}
+
+	/*
+	 * Avoid using 'update of [table.column]' syntax if the given aliasesToLock are actually all tables.
+	 *
+	 * The reason being, when the user attempts to create a query with both pagination and lock options,
+	 * the Oracle Dialect would simply rely on the `LIMIT_HANDLER` to decorate the original SQL as
+	 * `select .. (sql) where rownum <= ? ..`. Hence the `for update of` syntax will result in ORA-00904
+	 * (invalid identifier) in this kind of query.
+	 *
+	 * The generated for-update clause varies in below scenarios:
+	 *
+	 * 1. createQuery("from A a").setLockMode( "a", LockMode.PESSIMISTIC_WRITE )
+	 *     Result in `for update` only, because there is only one table in the query.
+	 *
+	 * 2. createQuery("from A a").setLockMode( LockMode.PESSIMISTIC_WRITE )
+	 *     Result in `for update` only, because the user did not intent to lock on specific alias at all.
+	 *
+	 * 3. createQuery("from A a join fetch a.b").setLockMode( "b", LockMode.PESSIMISTIC_WRITE )
+	 *     Result in `for update of b0_.id`, to only lock on the alias requested by the user.
+	 */
+	private boolean needToSpecifyAliasesInForUpdate(Set<String> tableAliasSet, String aliasesToLock) {
+		if ( StringHelper.isNotEmpty( aliasesToLock ) ) {
+			String[] tableAliasWithIdColumns = StringHelper.split(",", aliasesToLock);
+			HashSet<String> tableAliasToLock = new HashSet<>(  );
+			for (String tableAliasWithIdColumn : tableAliasWithIdColumns) {
+				int indexOfDot = tableAliasWithIdColumn.indexOf(".");
+				String tableAlias = indexOfDot == -1
+						? tableAliasWithIdColumn
+						: tableAliasWithIdColumn.substring( 0, indexOfDot );
+				tableAliasToLock.add( tableAlias );
+			}
+
+			return !tableAliasSet.equals(tableAliasToLock);
+		}
+
+		return false;
+	}
+
+	private String getForUpdateString(LockMode lockMode, int timeout, String aliases) {
+		switch ( lockMode ) {
+			case UPGRADE:
+				return getForUpdateString( aliases );
+			case PESSIMISTIC_READ:
+				return getReadLockString( aliases, timeout );
+			case PESSIMISTIC_WRITE:
+				return getWriteLockString( aliases, timeout );
+			case UPGRADE_NOWAIT:
+			case FORCE:
+			case PESSIMISTIC_FORCE_INCREMENT:
+				return getForUpdateNowaitString( aliases );
+			case UPGRADE_SKIPLOCKED:
+				return getForUpdateSkipLockedString( aliases );
+			default:
+				return "";
+		}
+	}
+
+	@Override
+	public String getReadLockString(String aliases, int timeout) {
+		return forUpdateFragment( aliases, timeout );
+	}
+
+	@Override
+	public String getWriteLockString(String aliases, int timeout) {
+		if ( timeout == LockOptions.SKIP_LOCKED ) {
+			return getForUpdateSkipLockedString( aliases );
+		}
+		else {
+			return forUpdateFragment( aliases, timeout );
+		}
+	}
+
+	private String forUpdateFragment(String aliases, int timeout) {
+		StringBuilder forUpdateFragment = new StringBuilder( getForUpdateString() );
+
+		// refer to https://docs.oracle.com/database/121/SQLRF/statements_10002.htm#i2126016
+		if ( StringHelper.isNotEmpty( aliases ) ) {
+			forUpdateFragment.append( " of " ).append( aliases );
+		}
+
+		if ( timeout == LockOptions.NO_WAIT ) {
+			forUpdateFragment.append( " nowait" );
+		}
+		else if ( timeout == LockOptions.SKIP_LOCKED ) {
+			forUpdateFragment.append( " skip locked" );
+		}
+		else if ( timeout > 0 ) {
+			// convert from milliseconds to seconds
+			final float seconds = timeout / 1000.0f;
+			forUpdateFragment.append( " wait " ).append( Math.round( seconds ) );
+		}
+
+		return forUpdateFragment.toString();
 	}
 
 	@Override

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

@@ -0,0 +1,58 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later.
+ * See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
+ */
+package org.hibernate.dialect;
+
+import static org.junit.Assert.assertEquals;
+
+import org.hibernate.LockMode;
+import org.hibernate.LockOptions;
+import org.junit.Test;
+
+public class Oracle10gDialectTestCase {
+
+	@Test
+	public void testGetForUpdateStringWithAllAliasesSpecified() {
+		Oracle10gDialect dialect = new Oracle10gDialect();
+		LockOptions lockOptions = new LockOptions();
+		lockOptions.setAliasSpecificLockMode( "tableAlias1", LockMode.PESSIMISTIC_WRITE );
+
+		String forUpdateClause = dialect.getForUpdateString( "tableAlias1", lockOptions );
+		assertEquals( " for update", forUpdateClause );
+
+		lockOptions.setAliasSpecificLockMode( "tableAlias2", LockMode.PESSIMISTIC_WRITE );
+		forUpdateClause = dialect.getForUpdateString( "tableAlias1,tableAlias2", lockOptions );
+		assertEquals( " for update", forUpdateClause );
+
+	}
+
+	@Test
+	public void testGetForUpdateStringWithoutAliasSpecified() {
+		Oracle10gDialect dialect = new Oracle10gDialect();
+		LockOptions lockOptions = new LockOptions();
+		lockOptions.setAliasSpecificLockMode( "tableAlias1", LockMode.PESSIMISTIC_WRITE );
+
+		String forUpdateClause = dialect.getForUpdateString( "", lockOptions );
+		assertEquals( " for update", forUpdateClause );
+	}
+
+	@Test
+	public void testGetForUpdateStringWithSomeAliasSpecified() {
+		Oracle10gDialect dialect = new Oracle10gDialect();
+		LockOptions lockOptions = new LockOptions();
+		lockOptions.setAliasSpecificLockMode( "tableAlias1", LockMode.PESSIMISTIC_WRITE );
+		lockOptions.setAliasSpecificLockMode( "tableAlias2", LockMode.PESSIMISTIC_WRITE );
+
+		String forUpdateClause = dialect.getForUpdateString( "tableAlias1", lockOptions );
+		assertEquals( " for update of tableAlias1", forUpdateClause );
+
+		lockOptions.setAliasSpecificLockMode( "tableAlias3", LockMode.PESSIMISTIC_WRITE );
+
+		forUpdateClause = dialect.getForUpdateString( "tableAlias1,tableAlias3", lockOptions );
+		assertEquals( " for update of tableAlias1,tableAlias3", forUpdateClause );
+	}
+
+}

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

@@ -0,0 +1,58 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later.
+ * See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
+ */
+package org.hibernate.dialect;
+
+import static org.junit.Assert.assertEquals;
+
+import org.hibernate.LockMode;
+import org.hibernate.LockOptions;
+import org.junit.Test;
+
+public class Oracle12cDialectTestCase {
+
+	@Test
+	public void testGetForUpdateStringWithAllAliasesSpecified() {
+		Oracle12cDialect dialect = new Oracle12cDialect();
+		LockOptions lockOptions = new LockOptions();
+		lockOptions.setAliasSpecificLockMode( "tableAlias1", LockMode.PESSIMISTIC_WRITE );
+
+		String forUpdateClause = dialect.getForUpdateString( "tableAlias1", lockOptions );
+		assertEquals( " for update", forUpdateClause );
+
+		lockOptions.setAliasSpecificLockMode( "tableAlias2", LockMode.PESSIMISTIC_WRITE );
+		forUpdateClause = dialect.getForUpdateString( "tableAlias1,tableAlias2", lockOptions );
+		assertEquals( " for update", forUpdateClause );
+
+	}
+
+	@Test
+	public void testGetForUpdateStringWithoutAliasSpecified() {
+		Oracle12cDialect dialect = new Oracle12cDialect();
+		LockOptions lockOptions = new LockOptions();
+		lockOptions.setAliasSpecificLockMode( "tableAlias1", LockMode.PESSIMISTIC_WRITE );
+
+		String forUpdateClause = dialect.getForUpdateString( "", lockOptions );
+		assertEquals( " for update", forUpdateClause );
+	}
+
+	@Test
+	public void testGetForUpdateStringWithSomeAliasSpecified() {
+		Oracle12cDialect dialect = new Oracle12cDialect();
+		LockOptions lockOptions = new LockOptions();
+		lockOptions.setAliasSpecificLockMode( "tableAlias1", LockMode.PESSIMISTIC_WRITE );
+		lockOptions.setAliasSpecificLockMode( "tableAlias2", LockMode.PESSIMISTIC_WRITE );
+
+		String forUpdateClause = dialect.getForUpdateString( "tableAlias1", lockOptions );
+		assertEquals( " for update of tableAlias1", forUpdateClause );
+
+		lockOptions.setAliasSpecificLockMode( "tableAlias3", LockMode.PESSIMISTIC_WRITE );
+
+		forUpdateClause = dialect.getForUpdateString( "tableAlias1,tableAlias3", lockOptions );
+		assertEquals( " for update of tableAlias1,tableAlias3", forUpdateClause );
+	}
+
+}

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

@@ -6,6 +6,8 @@
  */
 package org.hibernate.dialect;
 
+import org.hibernate.LockMode;
+import org.hibernate.LockOptions;
 import org.hibernate.hql.spi.id.AbstractMultiTableBulkIdStrategyImpl;
 
 import org.junit.Test;
@@ -37,4 +39,46 @@ public void testTemporaryTableNameTruncation() throws Exception {
 				temporaryTableName
 		);
 	}
+
+	@Test
+	public void testGetForUpdateStringWithAllAliasesSpecified() {
+		Oracle8iDialect dialect = new Oracle8iDialect();
+		LockOptions lockOptions = new LockOptions();
+		lockOptions.setAliasSpecificLockMode( "tableAlias1", LockMode.PESSIMISTIC_WRITE );
+
+		String forUpdateClause = dialect.getForUpdateString( "tableAlias1", lockOptions );
+		assertEquals( " for update", forUpdateClause );
+
+		lockOptions.setAliasSpecificLockMode( "tableAlias2", LockMode.PESSIMISTIC_WRITE );
+		forUpdateClause = dialect.getForUpdateString( "tableAlias1,tableAlias2", lockOptions );
+		assertEquals( " for update", forUpdateClause );
+
+	}
+
+	@Test
+	public void testGetForUpdateStringWithoutAliasSpecified() {
+		Oracle8iDialect dialect = new Oracle8iDialect();
+		LockOptions lockOptions = new LockOptions();
+		lockOptions.setAliasSpecificLockMode( "tableAlias1", LockMode.PESSIMISTIC_WRITE );
+
+		String forUpdateClause = dialect.getForUpdateString( "", lockOptions );
+		assertEquals( " for update", forUpdateClause );
+	}
+
+	@Test
+	public void testGetForUpdateStringWithSomeAliasSpecified() {
+		Oracle8iDialect dialect = new Oracle8iDialect();
+		LockOptions lockOptions = new LockOptions();
+		lockOptions.setAliasSpecificLockMode( "tableAlias1", LockMode.PESSIMISTIC_WRITE );
+		lockOptions.setAliasSpecificLockMode( "tableAlias2", LockMode.PESSIMISTIC_WRITE );
+
+		String forUpdateClause = dialect.getForUpdateString( "tableAlias1", lockOptions );
+		assertEquals( " for update of tableAlias1", forUpdateClause );
+
+		lockOptions.setAliasSpecificLockMode( "tableAlias3", LockMode.PESSIMISTIC_WRITE );
+
+		forUpdateClause = dialect.getForUpdateString( "tableAlias1,tableAlias3", lockOptions );
+		assertEquals( " for update of tableAlias1,tableAlias3", forUpdateClause );
+	}
+
 }