HHH-19925 - Locking root(s) should be based on select-clause, not from-clause

hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/DerbyDialect.java

@@ -59,6 +59,7 @@
 import org.hibernate.query.sqm.mutation.spi.SqmMultiTableInsertStrategy;
 import org.hibernate.query.sqm.mutation.spi.SqmMultiTableMutationStrategy;
 import org.hibernate.service.ServiceRegistry;
+import org.hibernate.spi.NavigablePath;
 import org.hibernate.sql.ast.SqlAstNodeRenderingMode;
 import org.hibernate.sql.ast.SqlAstTranslator;
 import org.hibernate.sql.ast.SqlAstTranslatorFactory;
@@ -85,6 +86,7 @@
 import java.sql.SQLException;
 import java.sql.Types;
 import java.util.Locale;
+import java.util.Set;
 
 import static org.hibernate.type.SqlTypes.BINARY;
 import static org.hibernate.type.SqlTypes.BLOB;
@@ -564,8 +566,9 @@ public LockingSupport getLockingSupport() {
 	protected LockingClauseStrategy buildLockingClauseStrategy(
 			PessimisticLockKind lockKind,
 			RowLockStrategy rowLockStrategy,
-			LockOptions lockOptions) {
-		return new DerbyLockingClauseStrategy( this, lockKind, rowLockStrategy, lockOptions );
+			LockOptions lockOptions,
+			Set<NavigablePath> rootPathsForLocking) {
+		return new DerbyLockingClauseStrategy( this, lockKind, rowLockStrategy, lockOptions, rootPathsForLocking );
 	}
 
 	@Override

hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/DerbyLegacyDialect.java

@@ -58,6 +58,7 @@
 import org.hibernate.query.sqm.mutation.spi.SqmMultiTableInsertStrategy;
 import org.hibernate.query.sqm.mutation.spi.SqmMultiTableMutationStrategy;
 import org.hibernate.service.ServiceRegistry;
+import org.hibernate.spi.NavigablePath;
 import org.hibernate.sql.ast.SqlAstNodeRenderingMode;
 import org.hibernate.sql.ast.SqlAstTranslator;
 import org.hibernate.sql.ast.SqlAstTranslatorFactory;
@@ -85,6 +86,7 @@
 import java.sql.DatabaseMetaData;
 import java.sql.SQLException;
 import java.sql.Types;
+import java.util.Set;
 
 import static org.hibernate.type.SqlTypes.BINARY;
 import static org.hibernate.type.SqlTypes.BLOB;
@@ -551,8 +553,9 @@ public boolean supportsCommentOn() {
 	protected LockingClauseStrategy buildLockingClauseStrategy(
 			PessimisticLockKind lockKind,
 			RowLockStrategy rowLockStrategy,
-			LockOptions lockOptions) {
-		return new DerbyLockingClauseStrategy( this, lockKind, rowLockStrategy, lockOptions );
+			LockOptions lockOptions,
+			Set<NavigablePath> rootPathsForLocking) {
+		return new DerbyLockingClauseStrategy( this, lockKind, rowLockStrategy, lockOptions, rootPathsForLocking );
 	}
 
 	@Override

hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/DerbyLockingClauseStrategy.java

@@ -7,10 +7,13 @@
 import org.hibernate.LockOptions;
 import org.hibernate.dialect.Dialect;
 import org.hibernate.dialect.RowLockStrategy;
+import org.hibernate.spi.NavigablePath;
 import org.hibernate.sql.ast.internal.PessimisticLockKind;
 import org.hibernate.sql.ast.internal.StandardLockingClauseStrategy;
 import org.hibernate.sql.ast.spi.SqlAppender;
 
+import java.util.Set;
+
 /**
  * StandardLockingClauseStrategy subclass, specific for Derby.
  *
@@ -21,8 +24,9 @@ public DerbyLockingClauseStrategy(
 			Dialect dialect,
 			PessimisticLockKind lockKind,
 			RowLockStrategy rowLockStrategy,
-			LockOptions lockOptions) {
-		super( dialect, lockKind, rowLockStrategy, lockOptions );
+			LockOptions lockOptions,
+			Set<NavigablePath> rootPathsForLocking) {
+		super( dialect, lockKind, rowLockStrategy, lockOptions, rootPathsForLocking );
 	}
 
 	@Override

hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/TeradataDialect.java

@@ -40,6 +40,7 @@
 import org.hibernate.query.sqm.mutation.internal.temptable.GlobalTemporaryTableMutationStrategy;
 import org.hibernate.query.sqm.mutation.spi.SqmMultiTableInsertStrategy;
 import org.hibernate.query.sqm.mutation.spi.SqmMultiTableMutationStrategy;
+import org.hibernate.spi.NavigablePath;
 import org.hibernate.sql.ForUpdateFragment;
 import org.hibernate.sql.ast.SqlAstTranslator;
 import org.hibernate.sql.ast.SqlAstTranslatorFactory;
@@ -65,6 +66,7 @@
 import java.sql.SQLException;
 import java.sql.Types;
 import java.util.Map;
+import java.util.Set;
 
 import static org.hibernate.exception.spi.TemplatedViolatedConstraintNameExtractor.extractUsingTemplate;
 import static org.hibernate.type.SqlTypes.BIGINT;
@@ -529,13 +531,14 @@ public LockingSupport getLockingSupport() {
 	protected LockingClauseStrategy buildLockingClauseStrategy(
 			PessimisticLockKind lockKind,
 			RowLockStrategy rowLockStrategy,
-			LockOptions lockOptions) {
+			LockOptions lockOptions,
+			Set<NavigablePath> rootPathsForLocking) {
 		if ( getVersion().isBefore( 14 ) ) {
 			return NonLockingClauseStrategy.NON_CLAUSE_STRATEGY;
 		}
 		// we'll reuse the StandardLockingClauseStrategy for the collecting
 		// aspect and just handle the special rendering in the SQL AST translator
-		return super.buildLockingClauseStrategy( lockKind, rowLockStrategy, lockOptions );
+		return super.buildLockingClauseStrategy( lockKind, rowLockStrategy, lockOptions, rootPathsForLocking );
 	}
 
 	@Override

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

@@ -114,6 +114,7 @@
 import org.hibernate.query.sqm.sql.SqmTranslatorFactory;
 import org.hibernate.service.ServiceRegistry;
 import org.hibernate.service.spi.ServiceRegistryImplementor;
+import org.hibernate.spi.NavigablePath;
 import org.hibernate.sql.ForUpdateFragment;
 import org.hibernate.sql.ast.SqlAstNodeRenderingMode;
 import org.hibernate.sql.ast.SqlAstTranslatorFactory;
@@ -2369,14 +2370,15 @@ public LockingClauseStrategy getLockingClauseStrategy(QuerySpec querySpec, LockO
 			default -> throw new IllegalStateException( "Should never happen due to checks above" );
 		}
 
-		return buildLockingClauseStrategy( lockKind, rowLockStrategy, lockOptions );
+		return buildLockingClauseStrategy( lockKind, rowLockStrategy, lockOptions, querySpec.getRootPathsForLocking() );
 	}
 
 	protected LockingClauseStrategy buildLockingClauseStrategy(
 			PessimisticLockKind lockKind,
 			RowLockStrategy rowLockStrategy,
-			LockOptions lockOptions) {
-		return new StandardLockingClauseStrategy( this, lockKind, rowLockStrategy, lockOptions );
+			LockOptions lockOptions,
+			Set<NavigablePath> rootPathsForLocking) {
+		return new StandardLockingClauseStrategy( this, lockKind, rowLockStrategy, lockOptions, rootPathsForLocking );
 	}
 
 	/**

hibernate-core/src/main/java/org/hibernate/dialect/lock/internal/SqlAstBasedLockingStrategy.java

@@ -73,8 +73,11 @@ public void lock(
 		lockOptions.setScope( lockScope );
 		lockOptions.setTimeOut( timeout );
 
-		final var rootQuerySpec = new QuerySpec( true );
 		final var entityPath = new NavigablePath( entityToLock.getRootPathName() );
+
+		final var rootQuerySpec = new QuerySpec( true );
+		rootQuerySpec.applyRootPathForLocking( entityPath );
+
 		final var idMapping = entityToLock.getIdentifierMapping();
 
 		// NOTE: there are 2 possible ways to handle the select list for the query...

hibernate-core/src/main/java/org/hibernate/loader/ast/internal/LoaderSelectBuilder.java

@@ -148,6 +148,8 @@ public static SelectStatement createSelectBySingleArrayParameter(
 		final var sqlAstCreationState = builder.createSqlAstCreationState( rootQuerySpec );
 
 		final var rootNavigablePath = new NavigablePath( loadable.getRootPathName() );
+		rootQuerySpec.applyRootPathForLocking( rootNavigablePath );
+
 		final var rootTableGroup =
 				builder.buildRootTableGroup( rootNavigablePath, rootQuerySpec, sqlAstCreationState );
 
@@ -464,6 +466,8 @@ private SelectStatement generateSelect() {
 		final var rootNavigablePath = new NavigablePath( loadable.getRootPathName() );
 
 		final var rootQuerySpec = new QuerySpec( true );
+		rootQuerySpec.applyRootPathForLocking( rootNavigablePath );
+
 		final var sqlAstCreationState = createSqlAstCreationState( rootQuerySpec );
 
 		final var rootTableGroup = buildRootTableGroup( rootNavigablePath, rootQuerySpec, sqlAstCreationState );
@@ -980,12 +984,11 @@ private SelectStatement generateSelect(SubselectFetch subselect) {
 		//		- so `loadable` is the owner entity-descriptor and the `partsToSelect` is the collection
 
 		assert loadable instanceof PluralAttributeMapping;
-
 		final var attributeMapping = (PluralAttributeMapping) loadable;
 
-		final var rootQuerySpec = new QuerySpec( true );
-
 		final var rootNavigablePath = new NavigablePath( loadable.getRootPathName() );
+		final var rootQuerySpec = new QuerySpec( true );
+		rootQuerySpec.applyRootPathForLocking( rootNavigablePath );
 
 		// We need to initialize the acronymMap based on subselect.getLoadingSqlAst() to avoid alias collisions
 		final var tableReferences = AliasCollector.getTableReferences( subselect.getLoadingSqlAst() );

hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/MatchingIdSelectionHelper.java

@@ -65,15 +65,11 @@
 public class MatchingIdSelectionHelper {
 	private static final Logger LOG = Logger.getLogger( MatchingIdSelectionHelper.class );
 
-	/**
-	 * @asciidoc
-	 *
-	 * Generates a query-spec for selecting all ids matching the restriction defined as part
-	 * of the user's update/delete query.  This query-spec is generally used:
-	 *
-	 * 		* to select all the matching ids via JDBC - see {@link MatchingIdSelectionHelper#selectMatchingIds}
-	 * 		* as a sub-query restriction to insert rows into an "id table"
-	 */
+	/// Generates a query-spec for selecting all ids matching the restriction defined as part
+	/// of the user's update/delete query.  This query-spec is generally used:
+	///
+	///  * to select all the matching ids via JDBC - see {@link MatchingIdSelectionHelper#selectMatchingIds}
+	///  * as a sub-query restriction to insert rows into an "id table"
 	public static SelectStatement generateMatchingIdSelectStatement(
 			EntityMappingType targetEntityDescriptor,
 			SqmDeleteOrUpdateStatement<?> sqmStatement,
@@ -109,14 +105,12 @@ public static SelectStatement generateMatchingIdSelectStatement(
 				mutatingTableGroup.getNavigablePath(),
 				mutatingTableGroup,
 				sqmConverter,
-				(selection, jdbcMapping) ->
-						domainResults.add(
-								new BasicResult<>(
-										selection.getValuesArrayPosition(),
-										null,
-										jdbcMapping
-								)
-						)
+				(selection, jdbcMapping) -> domainResults.add(
+					new BasicResult<>(
+							selection.getValuesArrayPosition(),
+							null,
+							jdbcMapping
+					) )
 		);
 		sqmConverter.getProcessingStateStack().pop();
 
@@ -133,15 +127,11 @@ public static SelectStatement generateMatchingIdSelectStatement(
 		return new SelectStatement( idSelectionQuery, domainResults );
 	}
 
-	/**
-	 * @asciidoc
-	 *
-	 * Generates a query-spec for selecting all ids matching the restriction defined as part
-	 * of the user's update/delete query.  This query-spec is generally used:
-	 *
-	 * 		* to select all the matching ids via JDBC - see {@link MatchingIdSelectionHelper#selectMatchingIds}
-	 * 		* as a sub-query restriction to insert rows into an "id table"
-	 */
+	/// Generates a query-spec for selecting all ids matching the restriction defined as part
+	/// of the user's update/delete query.  This query-spec is generally used:
+	///
+	/// 	* to select all the matching ids via JDBC - see {@link MatchingIdSelectionHelper#selectMatchingIds}
+	/// 	* as a sub-query restriction to insert rows into an "id table"
 	public static SqmSelectStatement<?> generateMatchingIdSelectStatement(
 			SqmDeleteOrUpdateStatement<?> sqmStatement,
 			EntityMappingType entityDescriptor) {
@@ -167,59 +157,6 @@ public static SqmSelectStatement<?> generateMatchingIdSelectStatement(
 				nodeBuilder
 		);
 	}
-//
-//	/**
-//	 * @asciidoc
-//	 *
-//	 * Generates a query-spec for selecting all ids matching the restriction defined as part
-//	 * of the user's update/delete query.  This query-spec is generally used:
-//	 *
-//	 * 		* to select all the matching ids via JDBC - see {@link MatchingIdSelectionHelper#selectMatchingIds}
-//	 * 		* as a sub-query restriction to insert rows into an "id table"
-//	 */
-//	public static QuerySpec generateMatchingIdSelectQuery(
-//			EntityMappingType targetEntityDescriptor,
-//			SqmDeleteOrUpdateStatement sqmStatement,
-//			DomainParameterXref domainParameterXref,
-//			Predicate restriction,
-//			MultiTableSqmMutationConverter sqmConverter,
-//			SessionFactoryImplementor sessionFactory) {
-//		final EntityDomainType entityDomainType = sqmStatement.getTarget().getModel();
-//		if ( LOG.isTraceEnabled() ) {
-//			LOG.tracef(
-//					"Starting generation of entity-id SQM selection - %s",
-//					entityDomainType.getHibernateEntityName()
-//			);
-//		}
-//
-//		final QuerySpec idSelectionQuery = new QuerySpec( true, 1 );
-//
-//		final TableGroup mutatingTableGroup = sqmConverter.getMutatingTableGroup();
-//		idSelectionQuery.getFromClause().addRoot( mutatingTableGroup );
-//
-//		targetEntityDescriptor.getIdentifierMapping().forEachSelectable(
-//				(position, selection) -> {
-//					final TableReference tableReference = mutatingTableGroup.resolveTableReference(
-//							mutatingTableGroup.getNavigablePath(),
-//							selection.getContainingTableExpression()
-//					);
-//					final Expression expression = sqmConverter.getSqlExpressionResolver().resolveSqlExpression(
-//							tableReference,
-//							selection
-//					);
-//					idSelectionQuery.getSelectClause().addSqlSelection(
-//							new SqlSelectionImpl(
-//									position,
-//									expression
-//							)
-//					);
-//				}
-//		);
-//
-//		idSelectionQuery.applyPredicate( restriction );
-//
-//		return idSelectionQuery;
-//	}
 
 	/**
 	 * Centralized selection of ids matching the restriction of the DELETE