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

Author: sebersole

hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java

@@ -2270,10 +2270,46 @@ private void collectRootPathsForLocking(SqmSelection<?> sqmSelection) {
 		if ( rootPathsForLockingCollector == null ) {
 			return;
 		}
-		if ( sqmSelection.getExpressible() instanceof EntityTypeImpl
-				&& sqmSelection.getSelectableNode() instanceof SqmPath<?> entityValuedPath ) {
-			rootPathsForLockingCollector.accept( entityValuedPath.getNavigablePath() );
+
+		collectRootPathsForLocking( sqmSelection.getSelectableNode() );
+	}
+
+	private void collectRootPathsForLocking(SqmSelectableNode<?> selectableNode) {
+		// roughly speaking we only care about 2 cases here:
+		//		1) entity path - the entity will be locked
+		//		2) scalar path - the entity from which the path originates will be locked
+		//
+		// note, however, that we need to account for both cases as the argument to a dynamic instantiation
+
+		if ( selectableNode instanceof SqmPath<?> selectedPath ) {
+			collectRootPathsForLocking( selectedPath );
+		}
+		else if ( selectableNode instanceof SqmDynamicInstantiation<?> dynamicInstantiation ) {
+			collectRootPathsForLocking( dynamicInstantiation );
+		}
+	}
+
+	private void collectRootPathsForLocking(SqmPath<?> selectedPath) {
+		assert rootPathsForLockingCollector != null;
+
+		if ( selectedPath == null ) {
+			// typically this comes from paths rooted in a CTE.
+			// regardless, without a path we cannot evaluate so just return.
+			return;
 		}
+
+		if ( selectedPath.getNodeType() instanceof EntityTypeImpl ) {
+			rootPathsForLockingCollector.accept( selectedPath.getNavigablePath() );
+		}
+		else {
+			collectRootPathsForLocking( selectedPath.getLhs() );
+		}
+	}
+
+	private void collectRootPathsForLocking(SqmDynamicInstantiation<?> dynamicInstantiation) {
+		dynamicInstantiation.getArguments().forEach( ( argument ) -> {
+			collectRootPathsForLocking( argument.getSelectableNode() );
+		} );
 	}
 
 	private void inferTargetPath(int index) {

hibernate-core/src/test/java/org/hibernate/orm/test/locking/LockingBasedOnSelectClauseTests.java

@@ -84,6 +84,10 @@ void testSubQueryHqlTranslation(SessionFactoryScope factoryScope) {
 		} );
 	}
 
+	private static final String BOOK_PATH = "org.hibernate.orm.test.locking.LockingBasedOnSelectClauseTests$Book";
+	private static final String BOOK_PATH_HQL = BOOK_PATH+ "(b)";
+	private static final String BOOK_AUTHOR_PATH_HQL = BOOK_PATH_HQL + ".author";
+
 	@Test
 	void testBasicHqlTranslation(SessionFactoryScope factoryScope) {
 		final HqlHelper.HqlTranslation hqlTranslation = HqlHelper.translateHql(
@@ -96,7 +100,56 @@ void testBasicHqlTranslation(SessionFactoryScope factoryScope) {
 		final SelectStatement selectAst = ( SelectStatement ) sqlAst;
 		assertThat( selectAst.getRootPathsForLocking() ).hasSize( 1 );
 		assertThat( selectAst.getRootPathsForLocking().get( 0 ).getFullPath() )
-				.isEqualTo( "org.hibernate.orm.test.locking.LockingBasedOnSelectClauseTests$Book(b).author");
+				.isEqualTo( BOOK_AUTHOR_PATH_HQL );
+	}
+
+	@Test
+	void testScalarHqlTranslation(SessionFactoryScope factoryScope) {
+		final HqlHelper.HqlTranslation hqlTranslation = HqlHelper.translateHql(
+				"select b.title from Book b",
+				factoryScope.getSessionFactory()
+		);
+
+		final Statement sqlAst = hqlTranslation.sqlAst();
+		assertThat( sqlAst ).isInstanceOf( SelectStatement.class );
+		final SelectStatement selectAst = ( SelectStatement ) sqlAst;
+		assertThat( selectAst.getRootPathsForLocking() ).hasSize( 1 );
+		assertThat( selectAst.getRootPathsForLocking().get( 0 ).getFullPath() )
+				.isEqualTo( BOOK_PATH_HQL );
+	}
+
+	@Test
+	void testScalarHqlTranslation2(SessionFactoryScope factoryScope) {
+		final HqlHelper.HqlTranslation hqlTranslation = HqlHelper.translateHql(
+				"select b.title, b.author from Book b",
+				factoryScope.getSessionFactory()
+		);
+
+		final Statement sqlAst = hqlTranslation.sqlAst();
+		assertThat( sqlAst ).isInstanceOf( SelectStatement.class );
+		final SelectStatement selectAst = ( SelectStatement ) sqlAst;
+		assertThat( selectAst.getRootPathsForLocking() ).hasSize( 2 );
+		assertThat( selectAst.getRootPathsForLocking().get( 0 ).getFullPath() )
+				.isEqualTo( BOOK_PATH_HQL );
+		assertThat( selectAst.getRootPathsForLocking().get( 1 ).getFullPath() )
+				.isEqualTo( BOOK_AUTHOR_PATH_HQL );
+	}
+
+	@Test
+	void testDynamicInstantiationHqlTranslation(SessionFactoryScope factoryScope) {
+		final HqlHelper.HqlTranslation hqlTranslation = HqlHelper.translateHql(
+				"select new list(b.title, b.author) from Book b",
+				factoryScope.getSessionFactory()
+		);
+
+		final Statement sqlAst = hqlTranslation.sqlAst();
+		assertThat( sqlAst ).isInstanceOf( SelectStatement.class );
+		final SelectStatement selectAst = ( SelectStatement ) sqlAst;
+		assertThat( selectAst.getRootPathsForLocking() ).hasSize( 2 );
+		assertThat( selectAst.getRootPathsForLocking().get( 0 ).getFullPath() )
+				.isEqualTo( BOOK_PATH_HQL );
+		assertThat( selectAst.getRootPathsForLocking().get( 1 ).getFullPath() )
+				.isEqualTo( BOOK_AUTHOR_PATH_HQL );
 	}
 
 	@Test
@@ -109,25 +162,25 @@ void testLoadingTranslation(SessionFactoryScope factoryScope) {
 		);
 		assertThat( translation.sqlAst().getRootPathsForLocking() ).hasSize( 1 );
 		assertThat( translation.sqlAst().getRootPathsForLocking().get( 0 ).getFullPath() )
-				.isEqualTo( "org.hibernate.orm.test.locking.LockingBasedOnSelectClauseTests$Book");
+				.isEqualTo( BOOK_PATH );
 	}
 
 	@Entity(name="Book")
 	@Table(name="books")
 	public static class Book {
 		@Id
 		private Integer id;
-		private String name;
+		private String title;
 		@ManyToOne(fetch = FetchType.LAZY)
 		@JoinColumn(name = "author_fk")
 		private Author author;
 
 		public Book() {
 		}
 
-		public Book(Integer id, String name, Author author) {
+		public Book(Integer id, String title, Author author) {
 			this.id = id;
-			this.name = name;
+			this.title = title;
 			this.author = author;
 		}
 	}