From c07eec02b169f96f87a51958f14d4984adad808e Mon Sep 17 00:00:00 2001 From: Steve Ebersole Date: Wed, 12 Nov 2025 17:48:57 -0700 Subject: [PATCH 1/3] HHH-19925 - Locking root(s) should be based on select-clause, not from-clause --- .../internal/SqlAstBasedLockingStrategy.java | 2 +- .../ast/internal/AbstractNaturalIdLoader.java | 2 +- .../internal/DatabaseSnapshotExecutor.java | 2 +- .../ast/internal/LoaderSelectBuilder.java | 7 +- .../internal/MatchingIdSelectionHelper.java | 97 +---- .../cte/AbstractCteMutationHandler.java | 2 +- .../internal/cte/CteInsertHandler.java | 2 +- .../temptable/TableBasedInsertHandler.java | 3 +- .../sqm/sql/BaseSqmToSqlAstConverter.java | 31 +- .../sql/ast/spi/AbstractSqlAstTranslator.java | 8 +- .../sql/ast/tree/select/SelectStatement.java | 39 +- .../sql/exec/internal/lock/TableLock.java | 2 +- .../LockingBasedOnSelectClauseTests.java | 150 ++++++++ .../hibernate/testing/util/ast/HqlHelper.java | 357 ++++++++++++++++++ .../testing/util/ast/LoadingAstHelper.java | 85 +++++ 15 files changed, 679 insertions(+), 110 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/locking/LockingBasedOnSelectClauseTests.java create mode 100644 hibernate-testing/src/main/java/org/hibernate/testing/util/ast/HqlHelper.java create mode 100644 hibernate-testing/src/main/java/org/hibernate/testing/util/ast/LoadingAstHelper.java diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/lock/internal/SqlAstBasedLockingStrategy.java b/hibernate-core/src/main/java/org/hibernate/dialect/lock/internal/SqlAstBasedLockingStrategy.java index 5aa2f194e7a1..6c483c05c25b 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/lock/internal/SqlAstBasedLockingStrategy.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/lock/internal/SqlAstBasedLockingStrategy.java @@ -154,7 +154,7 @@ public void lock( ); } - final var selectStatement = new SelectStatement( rootQuerySpec, List.of( idResult ) ); + final var selectStatement = new SelectStatement( rootQuerySpec, List.of( idResult ), List.of( entityPath ) ); final JdbcSelect selectOperation = session.getDialect().getSqlAstTranslatorFactory() .buildSelectTranslator( factory, selectStatement ) diff --git a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/AbstractNaturalIdLoader.java b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/AbstractNaturalIdLoader.java index b8e5529868fa..4fb3414e6548 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/AbstractNaturalIdLoader.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/AbstractNaturalIdLoader.java @@ -215,7 +215,7 @@ public Object resolveNaturalIdToId(Object naturalIdValue, SharedSessionContractI return executeNaturalIdQuery( naturalIdValue, LockOptions.NONE, - new SelectStatement( rootQuerySpec, singletonList( domainResult ) ), + new SelectStatement( rootQuerySpec, singletonList( domainResult ), List.of() ), rootTableGroup, rootQuerySpec::applyPredicate, sqlAstCreationState, diff --git a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/DatabaseSnapshotExecutor.java b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/DatabaseSnapshotExecutor.java index 7571ee6b0e76..c6dd25b1c55e 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/DatabaseSnapshotExecutor.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/DatabaseSnapshotExecutor.java @@ -128,7 +128,7 @@ class DatabaseSnapshotExecutor { } ); - final var selectStatement = new SelectStatement( rootQuerySpec, domainResults ); + final var selectStatement = new SelectStatement( rootQuerySpec, domainResults, List.of() ); jdbcSelect = sessionFactory.getJdbcServices().getJdbcEnvironment().getSqlAstTranslatorFactory() .buildSelectTranslator( sessionFactory, selectStatement ) diff --git a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/LoaderSelectBuilder.java b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/LoaderSelectBuilder.java index 654d9aaedfe4..de1a1a0fcd0c 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/LoaderSelectBuilder.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/LoaderSelectBuilder.java @@ -177,7 +177,7 @@ public static SelectStatement createSelectBySingleArrayParameter( builder.applyFiltering( rootQuerySpec, rootTableGroup, (Restrictable) loadable, sqlAstCreationState ); } - return new SelectStatement( rootQuerySpec, domainResults ); + return new SelectStatement( rootQuerySpec, domainResults, List.of( rootNavigablePath ) ); } private static void applyArrayParamRestriction( @@ -505,7 +505,7 @@ else if ( cachedDomainResult != null ) { applyFiltering( rootQuerySpec, rootTableGroup, (Restrictable) loadable, sqlAstCreationState ); } - return new SelectStatement( rootQuerySpec, domainResults ); + return new SelectStatement( rootQuerySpec, domainResults, List.of( rootNavigablePath ) ); } private List> buildRequestedDomainResults( @@ -1024,7 +1024,8 @@ private SelectStatement generateSelect(SubselectFetch subselect) { rootTableGroup, sqlAstCreationState ) - ) + ), + singletonList( rootNavigablePath ) ); } diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/MatchingIdSelectionHelper.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/MatchingIdSelectionHelper.java index 3999e5004ca1..674fea3e50dc 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/MatchingIdSelectionHelper.java +++ b/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(); @@ -130,18 +124,14 @@ public static SelectStatement generateMatchingIdSelectStatement( sqmConverter ); - return new SelectStatement( idSelectionQuery, domainResults ); + return new SelectStatement( idSelectionQuery, domainResults, List.of() ); } - /** - * @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 diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/cte/AbstractCteMutationHandler.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/cte/AbstractCteMutationHandler.java index d36d4c9b1892..e3b46a2365ee 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/cte/AbstractCteMutationHandler.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/cte/AbstractCteMutationHandler.java @@ -138,7 +138,7 @@ public AbstractCteMutationHandler( // Create the main query spec that will return the count of final QuerySpec querySpec = new QuerySpec( true, 1 ); final List> domainResults = new ArrayList<>( 1 ); - final SelectStatement statement = new SelectStatement( querySpec, domainResults ); + final SelectStatement statement = new SelectStatement( querySpec, domainResults, List.of() ); final JdbcServices jdbcServices = factory.getJdbcServices(); final SqlAstTranslator translator = jdbcServices.getJdbcEnvironment() .getSqlAstTranslatorFactory() diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/cte/CteInsertHandler.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/cte/CteInsertHandler.java index ffe734e4e842..307d9cb55b4f 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/cte/CteInsertHandler.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/cte/CteInsertHandler.java @@ -291,7 +291,7 @@ public CteInsertHandler( // Create the main query spec that will return the count of rows final QuerySpec querySpec = new QuerySpec( true, 1 ); final List> domainResults = new ArrayList<>( 1 ); - final SelectStatement statement = new SelectStatement( querySpec, domainResults ); + final SelectStatement statement = new SelectStatement( querySpec, domainResults, List.of() ); final CteStatement entityCte; if ( additionalInsertValues.requiresRowNumberIntermediate() ) { diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/temptable/TableBasedInsertHandler.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/temptable/TableBasedInsertHandler.java index 77dd84d9e8f5..05b1fc631c52 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/temptable/TableBasedInsertHandler.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/temptable/TableBasedInsertHandler.java @@ -593,7 +593,8 @@ private RootTableInserter createRootTableInserter( null, false ) - ) + ), + List.of() ); temporaryTableIdentitySelect = jdbcServices.getJdbcEnvironment() .getSqlAstTranslatorFactory() diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java index e372b307d99f..7fdb293d83cd 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java @@ -1640,11 +1640,12 @@ public SelectStatement visitSelectStatement(SqmSelectStatement statement) { final QueryPart queryPart = visitQueryPart( statement.getQueryPart() ); final List> domainResults = queryPart.isRoot() ? this.domainResults : emptyList(); try { - return new SelectStatement( cteContainer, queryPart, domainResults ); + return new SelectStatement( cteContainer, queryPart, domainResults, rootPathsForLocking ); } finally { this.currentSqmStatement = oldSqmStatement; this.cteContainer = oldCteContainer; + rootPathsForLocking = null; } } @@ -1753,7 +1754,7 @@ public CteStatement visitCteStatement(SqmCteStatement sqmCteStatement) { final CteStatement cteStatement = new CteStatement( cteTable, - new SelectStatement( subCteContainer, group, emptyList() ), + new SelectStatement( subCteContainer, group, emptyList(), emptyList() ), sqmCteStatement.getMaterialization(), sqmCteStatement.getSearchClauseKind(), visitSearchBySpecifications( cteTable, sqmCteStatement.getSearchBySpecifications() ), @@ -2205,11 +2206,22 @@ private TableGroup findTableGroupByPath(NavigablePath navigablePath) { return getFromClauseAccess().getTableGroup( navigablePath ); } + private List rootPathsForLocking; + private Consumer rootPathsForLockingCollector; + @Override public SelectClause visitSelectClause(SqmSelectClause selectClause) { currentClauseStack.push( Clause.SELECT ); try { final SelectClause sqlSelectClause = currentQuerySpec().getSelectClause(); + if ( sqmQueryPartStack.depth() == 1 ) { + rootPathsForLockingCollector = (navigablePath) -> { + if ( rootPathsForLocking == null ) { + rootPathsForLocking = new ArrayList<>(); + } + rootPathsForLocking.add( navigablePath ); + }; + } if ( selectClause == null ) { final SqmFrom implicitSelection = determineImplicitSelection( (SqmQuerySpec) getCurrentSqmQueryPart() ); visitSelection( 0, new SqmSelection<>( implicitSelection, implicitSelection.nodeBuilder() ) ); @@ -2224,6 +2236,7 @@ public SelectClause visitSelectClause(SqmSelectClause selectClause) { return sqlSelectClause; } finally { + rootPathsForLockingCollector = null; currentClauseStack.pop(); } } @@ -2243,6 +2256,8 @@ public Void visitSelection(SqmSelection sqmSelection) { } private void visitSelection(int index, SqmSelection sqmSelection) { + collectRootPathsForLocking( sqmSelection ); + inferTargetPath( index ); callResultProducers( resultProducers( sqmSelection ) ); if ( statement instanceof SqmInsertSelectStatement @@ -2251,6 +2266,16 @@ && contributesToTopLevelSelectClause() ) { } } + private void collectRootPathsForLocking(SqmSelection sqmSelection) { + if ( rootPathsForLockingCollector == null ) { + return; + } + if ( sqmSelection.getExpressible() instanceof EntityTypeImpl + && sqmSelection.getSelectableNode() instanceof SqmPath entityValuedPath ) { + rootPathsForLockingCollector.accept( entityValuedPath.getNavigablePath() ); + } + } + private void inferTargetPath(int index) { // Only infer the type on the "top level" select clauses // todo: add WriteExpression handling @@ -7129,7 +7154,7 @@ public SelectStatement visitSubQueryExpression(SqmSubQuery sqmSubQuery) { final QueryPart queryPart = visitQueryPart( sqmSubQuery.getQueryPart() ); currentlyProcessingJoin = oldJoin; this.cteContainer = oldCteContainer; - return new SelectStatement( cteContainer, queryPart, emptyList() ); + return new SelectStatement( cteContainer, queryPart, emptyList(), emptyList() ); } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/AbstractSqlAstTranslator.java b/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/AbstractSqlAstTranslator.java index 2d3d3a645447..2deaa363d3b1 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/AbstractSqlAstTranslator.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/AbstractSqlAstTranslator.java @@ -6492,6 +6492,7 @@ protected Predicate determineLateralEmulationPredicate(TableGroup tableGroup) { new SelectStatement( statement, new QueryGroup( false, SetOperator.INTERSECT, queryParts ), + emptyList(), emptyList() ), false, @@ -6540,7 +6541,7 @@ protected Predicate determineLateralEmulationPredicate(TableGroup tableGroup) { ); return new ExistsPredicate( - new SelectStatement( statement, existsQuery, emptyList() ), + new SelectStatement( statement, existsQuery, emptyList(), emptyList() ), false, booleanType ); @@ -6583,7 +6584,7 @@ protected Predicate determineLateralEmulationPredicate(TableGroup tableGroup) { ); final ExistsPredicate existsPredicate = new ExistsPredicate( - new SelectStatement( statement, existsQuery, emptyList() ), + new SelectStatement( statement, existsQuery, emptyList(), emptyList() ), false, booleanType ); @@ -6701,7 +6702,7 @@ protected Predicate determineLateralEmulationPredicate(TableGroup tableGroup) { List.of( existsPredicate, new BetweenPredicate( - new SelectStatement( statement, countQuery, emptyList() ), + new SelectStatement( statement, countQuery, emptyList(), emptyList() ), countLower, countUpper, false, @@ -6764,6 +6765,7 @@ private SelectStatement stripToSelectClause(SelectStatement statement) { return new SelectStatement( statement, stripToSelectClause( statement.getQueryPart() ), + emptyList(), emptyList() ); } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/select/SelectStatement.java b/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/select/SelectStatement.java index a15043918750..1f896070831b 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/select/SelectStatement.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/select/SelectStatement.java @@ -4,12 +4,10 @@ */ package org.hibernate.sql.ast.tree.select; -import java.util.Collections; -import java.util.List; - import org.hibernate.metamodel.mapping.JdbcMapping; import org.hibernate.metamodel.mapping.JdbcMappingContainer; import org.hibernate.query.sqm.sql.internal.DomainResultProducer; +import org.hibernate.spi.NavigablePath; import org.hibernate.sql.ast.SqlAstWalker; import org.hibernate.sql.ast.spi.SqlExpressionResolver; import org.hibernate.sql.ast.spi.SqlSelection; @@ -22,28 +20,37 @@ import org.hibernate.sql.results.graph.basic.BasicResult; import org.hibernate.type.spi.TypeConfiguration; +import java.util.Collections; +import java.util.List; + /** * @author Steve Ebersole */ public class SelectStatement extends AbstractStatement implements SqlAstNode, Expression, DomainResultProducer { private final QueryPart queryPart; private final List> domainResults; + private final List rootPathsForLocking; public SelectStatement(QueryPart queryPart) { - this( queryPart, Collections.emptyList() ); + this( queryPart, Collections.emptyList(), Collections.emptyList() ); } - public SelectStatement(QueryPart queryPart, List> domainResults) { - this( null, queryPart, domainResults ); + public SelectStatement( + QueryPart queryPart, + List> domainResults, + List rootPathsForLocking) { + this( null, queryPart, domainResults, rootPathsForLocking ); } public SelectStatement( CteContainer cteContainer, QueryPart queryPart, - List> domainResults) { + List> domainResults, + List rootPathsForLocking) { super( cteContainer ); this.queryPart = queryPart; this.domainResults = domainResults; + this.rootPathsForLocking = rootPathsForLocking == null ? Collections.emptyList() : rootPathsForLocking; } @Override @@ -63,6 +70,12 @@ public List> getDomainResultDescriptors() { return domainResults; } + /// List of [NavigablePath] references to be considered roots + /// for locking purposes. + public List getRootPathsForLocking() { + return rootPathsForLocking; + } + @Override public void accept(SqlAstWalker walker) { walker.visitSelectStatement( this ); @@ -122,13 +135,11 @@ public void applySqlSelections(DomainResultCreationState creationState) { public JdbcMappingContainer getExpressionType() { final SelectClause selectClause = queryPart.getFirstQuerySpec().getSelectClause(); final List sqlSelections = selectClause.getSqlSelections(); - switch ( sqlSelections.size() ) { - case 1: - return sqlSelections.get( 0 ).getExpressionType(); - default: - // todo (6.0): At some point we should create an ArrayTupleType and return that - case 0: - return null; + if ( sqlSelections.size() == 1 ) { + return sqlSelections.get( 0 ).getExpressionType(); + } + else { + return null; } } } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/exec/internal/lock/TableLock.java b/hibernate-core/src/main/java/org/hibernate/sql/exec/internal/lock/TableLock.java index 23d594fb8ff4..ee05802d6f08 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/exec/internal/lock/TableLock.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/exec/internal/lock/TableLock.java @@ -238,7 +238,7 @@ private void applyCompositeKeyRestriction(List entityKeys, SharedSess public void performActions(Map entityDetailsMap, QueryOptions lockingQueryOptions, SharedSessionContractImplementor session) { final var sessionFactory = session.getSessionFactory(); final var jdbcServices = sessionFactory.getJdbcServices(); - final var selectStatement = new SelectStatement( querySpec, domainResults ); + final var selectStatement = new SelectStatement( querySpec, domainResults, List.of( rootPath ) ); final List results = jdbcServices.getJdbcSelectExecutor() .executeQuery( diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/locking/LockingBasedOnSelectClauseTests.java b/hibernate-core/src/test/java/org/hibernate/orm/test/locking/LockingBasedOnSelectClauseTests.java new file mode 100644 index 000000000000..afff6d8bc6b0 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/locking/LockingBasedOnSelectClauseTests.java @@ -0,0 +1,150 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright Red Hat Inc. and Hibernate Authors + */ +package org.hibernate.orm.test.locking; + +import jakarta.persistence.Entity; +import jakarta.persistence.FetchType; +import jakarta.persistence.Id; +import jakarta.persistence.JoinColumn; +import jakarta.persistence.LockModeType; +import jakarta.persistence.ManyToOne; +import jakarta.persistence.Table; +import org.hibernate.sql.ast.tree.Statement; +import org.hibernate.sql.ast.tree.select.SelectStatement; +import org.hibernate.testing.orm.junit.DomainModel; +import org.hibernate.testing.orm.junit.JiraKey; +import org.hibernate.testing.orm.junit.SessionFactory; +import org.hibernate.testing.orm.junit.SessionFactoryScope; +import org.hibernate.testing.util.ast.HqlHelper; +import org.hibernate.testing.util.ast.LoadingAstHelper; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * @author Steve Ebersole + */ +@SuppressWarnings("JUnitMalformedDeclaration") +@JiraKey("HHH-19925") +@DomainModel(annotatedClasses = { + LockingBasedOnSelectClauseTests.Book.class, + LockingBasedOnSelectClauseTests.Author.class +}) +@SessionFactory +public class LockingBasedOnSelectClauseTests { + @BeforeAll + void setUp(SessionFactoryScope factoryScope) { + factoryScope.inTransaction( (session) -> { + var king = new Author( 1, "Stephen King" ); + var darkTower = new Book( 1, "The Dark Tower", king ); + session.persist( king ); + session.persist( darkTower ); + } ); + } + + @AfterAll + void tearDown(SessionFactoryScope factoryScope) { + factoryScope.dropData(); + } + + @Test + void testBasicHqlUsage(SessionFactoryScope factoryScope) { + factoryScope.inTransaction( (session) -> { + session.createQuery( "select b.author from Book b" ) + .setLockMode( LockModeType.PESSIMISTIC_WRITE ) + .list(); + // NOTE : The correct outcome here is for the authors table to be locked as the root selection. + // However, that is not what happens today - atm, the books table is locked (as the root + // of the from-clause). + // So ideally we want to make sure that - + // * authors is locked + // * books is not + // Unfortunately we cannot even craft a good assertion here because some of the dialects + // will lock both. + +// TransactionUtil.assertRowLock( +// factoryScope, +// "authors", +// "name", +// "id", +// 1, +// session.getDialect().getLockingSupport().getMetadata().getWriteRowLockStrategy() == RowLockStrategy.NONE +// ); + } ); + } + + @Test + void testSubQueryHqlTranslation(SessionFactoryScope factoryScope) { + factoryScope.inTransaction( (session) -> { + session.createQuery( "from Book b where b.id in (select id from Book)" ).list(); + } ); + } + + @Test + void testBasicHqlTranslation(SessionFactoryScope factoryScope) { + final HqlHelper.HqlTranslation hqlTranslation = HqlHelper.translateHql( + "select 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( 1 ); + assertThat( selectAst.getRootPathsForLocking().get( 0 ).getFullPath() ) + .isEqualTo( "org.hibernate.orm.test.locking.LockingBasedOnSelectClauseTests$Book(b).author"); + } + + @Test + void testLoadingTranslation(SessionFactoryScope factoryScope) { + var entityDescriptor = factoryScope.getSessionFactory().getMappingMetamodel().getEntityDescriptor( Book.class ); + var translation = LoadingAstHelper.translateLoading( + entityDescriptor, + 1, + factoryScope.getSessionFactory() + ); + assertThat( translation.sqlAst().getRootPathsForLocking() ).hasSize( 1 ); + assertThat( translation.sqlAst().getRootPathsForLocking().get( 0 ).getFullPath() ) + .isEqualTo( "org.hibernate.orm.test.locking.LockingBasedOnSelectClauseTests$Book"); + } + + @Entity(name="Book") + @Table(name="books") + public static class Book { + @Id + private Integer id; + private String name; + @ManyToOne(fetch = FetchType.LAZY) + @JoinColumn(name = "author_fk") + private Author author; + + public Book() { + } + + public Book(Integer id, String name, Author author) { + this.id = id; + this.name = name; + this.author = author; + } + } + + @Entity(name="Author") + @Table(name="authors") + public static class Author { + @Id + private Integer id; + private String name; + + public Author() { + } + + public Author(Integer id, String name) { + this.id = id; + this.name = name; + } + } +} diff --git a/hibernate-testing/src/main/java/org/hibernate/testing/util/ast/HqlHelper.java b/hibernate-testing/src/main/java/org/hibernate/testing/util/ast/HqlHelper.java new file mode 100644 index 000000000000..e3f93cd99048 --- /dev/null +++ b/hibernate-testing/src/main/java/org/hibernate/testing/util/ast/HqlHelper.java @@ -0,0 +1,357 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright Red Hat Inc. and Hibernate Authors + */ +package org.hibernate.testing.util.ast; + +import org.hibernate.Session; +import org.hibernate.engine.spi.LoadQueryInfluencers; +import org.hibernate.engine.spi.SessionFactoryImplementor; +import org.hibernate.engine.spi.SharedSessionContractImplementor; +import org.hibernate.metamodel.mapping.MappingModelExpressible; +import org.hibernate.query.ParameterMetadata; +import org.hibernate.query.hql.HqlTranslator; +import org.hibernate.query.internal.ParameterMetadataImpl; +import org.hibernate.query.internal.QueryParameterBindingsImpl; +import org.hibernate.query.spi.HqlInterpretation; +import org.hibernate.query.spi.ParameterMetadataImplementor; +import org.hibernate.query.spi.QueryOptions; +import org.hibernate.query.spi.QueryParameterBinding; +import org.hibernate.query.spi.QueryParameterImplementor; +import org.hibernate.query.sqm.internal.DomainParameterXref; +import org.hibernate.query.sqm.internal.SqmUtil; +import org.hibernate.query.sqm.spi.SqmParameterMappingModelResolutionAccess; +import org.hibernate.query.sqm.sql.SqmTranslation; +import org.hibernate.query.sqm.sql.SqmTranslator; +import org.hibernate.query.sqm.sql.SqmTranslatorFactory; +import org.hibernate.query.sqm.tree.SqmStatement; +import org.hibernate.query.sqm.tree.delete.SqmDeleteStatement; +import org.hibernate.query.sqm.tree.expression.SqmParameter; +import org.hibernate.query.sqm.tree.insert.SqmInsertStatement; +import org.hibernate.query.sqm.tree.select.SqmSelectStatement; +import org.hibernate.query.sqm.tree.update.SqmUpdateStatement; +import org.hibernate.sql.ast.SqlAstTranslator; +import org.hibernate.sql.ast.SqlAstTranslatorFactory; +import org.hibernate.sql.ast.tree.Statement; +import org.hibernate.sql.ast.tree.delete.DeleteStatement; +import org.hibernate.sql.ast.tree.insert.InsertStatement; +import org.hibernate.sql.ast.tree.select.SelectStatement; +import org.hibernate.sql.ast.tree.update.UpdateStatement; +import org.hibernate.sql.exec.internal.JdbcOperationQueryDelete; +import org.hibernate.sql.exec.internal.JdbcOperationQuerySelect; +import org.hibernate.sql.exec.internal.JdbcOperationQueryUpdate; +import org.hibernate.sql.exec.spi.JdbcOperation; +import org.hibernate.sql.exec.spi.JdbcOperationQueryInsert; +import org.hibernate.sql.exec.spi.JdbcParameterBindings; + +/// Utilities for helping test HQL translation +/// +/// @author Steve Ebersole +public class HqlHelper { + + /// Translation details about a particular HQL + /// + /// @param hql The translated HQL + /// @param sqm The corresponding SQM AST + /// @param sql The corresponding SQL + /// @param sqlAst The corresponding SQL AST + /// @param parameterMetadata Details about any query parameters + public record HqlTranslation( + String hql, + SqmStatement sqm, + String sql, + Statement sqlAst, + ParameterMetadata parameterMetadata) { + } + + /// Performs the translation, returning the details. Delegates to {@linkplain #translateHql(String, Class, SessionFactoryImplementor)} + /// passing {@code Object[]} as the expected result type. + public static HqlTranslation translateHql(String hql, SessionFactoryImplementor sessionFactory) { + return translateHql( hql, Object[].class, sessionFactory ); + } + + /// Performs the translation, returning the details. + @SuppressWarnings("rawtypes") + public static HqlTranslation translateHql(String hql, Class resultType, SessionFactoryImplementor sessionFactory) { + final HqlTranslator hqlTranslator = sessionFactory.getQueryEngine().getHqlTranslator(); + final SqmStatement sqmAst = hqlTranslator.translate( hql, resultType ); + + if ( sqmAst instanceof SqmSelectStatement sqmSelect ) { + //noinspection unchecked + return new SqmSelectInterpreter<>( hql, sessionFactory ).interpret( sqmSelect, sessionFactory ); + } + else if ( sqmAst instanceof SqmDeleteStatement sqmDelete ) { + //noinspection unchecked + return new SqmDeleteInterpreter<>( hql, sessionFactory ).interpret( sqmDelete, sessionFactory ); + } + else if ( sqmAst instanceof SqmUpdateStatement sqmUpdate ) { + //noinspection unchecked + return new SqmUpdateInterpreter<>( hql, sessionFactory ).interpret( sqmUpdate, sessionFactory ); + } + else if ( sqmAst instanceof SqmInsertStatement sqmInsert ) { + //noinspection unchecked + return new SqmInsertInterpreter<>( hql, sessionFactory ).interpret( sqmInsert, sessionFactory ); + } + + throw new UnsupportedOperationException( "Unexpected SQM type from HQL - " + sqmAst.getClass().getName() ); + } + + + // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + // internals + + private static abstract class SqmInterpreter, S extends Statement, J extends JdbcOperation> { + protected final String hql; + protected final SessionFactoryImplementor sessionFactory; + + public SqmInterpreter(String hql, SessionFactoryImplementor sessionFactory) { + this.hql = hql; + this.sessionFactory = sessionFactory; + } + + public HqlTranslation interpret(T sqmAst, SessionFactoryImplementor sessionFactory) { + final HqlInterpretation hqlInterpretation = createHqlInterpretation( sqmAst ); + + final QueryParameterBindingsImpl domainParameterBindings = QueryParameterBindingsImpl.from( + hqlInterpretation.getParameterMetadata(), + sessionFactory + ); + + final SqmTranslator sqmTranslator = createSqmTranslator( hqlInterpretation, domainParameterBindings ); + final SqmTranslation sqmTranslation = sqmTranslator.translate(); + + final SqlAstTranslator sqlAstTranslator = createSqlAstTranslator( sqmTranslation ); + + final J jdbcOperation = sessionFactory.fromSession( (session) -> { + final JdbcParameterBindings jdbcParameterBindings = createJdbcParameterBindings( + sqmTranslation, + hqlInterpretation.getDomainParameterXref(), + domainParameterBindings, + session + ); + return sqlAstTranslator.translate( jdbcParameterBindings, QueryOptions.NONE ); + } ); + + return new HqlTranslation( + hql, + hqlInterpretation.getSqmStatement(), + jdbcOperation.getSqlString(), + sqmTranslation.getSqlAst(), + hqlInterpretation.getParameterMetadata() + ); + } + + private HqlInterpretation createHqlInterpretation(T sqmAst) { + final ParameterMetadataImplementor parameterMetadata; + final DomainParameterXref domainParameterXref; + + if ( sqmAst.getSqmParameters().isEmpty() ) { + domainParameterXref = DomainParameterXref.EMPTY; + parameterMetadata = ParameterMetadataImpl.EMPTY; + } + else { + domainParameterXref = DomainParameterXref.from( sqmAst ); + parameterMetadata = new ParameterMetadataImpl( domainParameterXref.getQueryParameters() ); + } + + return new NonCopyingHqlInterpretationImpl<>( sqmAst, parameterMetadata, domainParameterXref ); + } + + protected abstract SqmTranslator createSqmTranslator( + HqlInterpretation hqlInterpretation, + QueryParameterBindingsImpl parameterBindings); + + protected abstract SqlAstTranslator createSqlAstTranslator( + SqmTranslation sqmTranslation); + + private JdbcParameterBindings createJdbcParameterBindings( + SqmTranslation sqmTranslation, + DomainParameterXref domainParameterXref, + QueryParameterBindingsImpl parameterBindings, + Session session) { + return SqmUtil.createJdbcParameterBindings( + parameterBindings, + domainParameterXref, + SqmUtil.generateJdbcParamsXref( + domainParameterXref, + sqmTranslation::getJdbcParamsBySqmParam + ), + new SqmParameterMappingModelResolutionAccess() { + @Override + public MappingModelExpressible getResolvedMappingModelType(SqmParameter parameter) { + final QueryParameterImplementor domainParam = domainParameterXref.getQueryParameter( parameter ); + final QueryParameterBinding binding = parameterBindings.getBinding( domainParam ); + //noinspection unchecked + return (MappingModelExpressible) binding.getType(); + } + }, + session.unwrap( SharedSessionContractImplementor.class ) + ); + } + } + + private static class SqmSelectInterpreter extends SqmInterpreter, SelectStatement, JdbcOperationQuerySelect> { + public SqmSelectInterpreter( + String hql, + SessionFactoryImplementor sessionFactory) { + super( hql, sessionFactory ); + } + + @Override + protected SqmTranslator createSqmTranslator( + HqlInterpretation hqlInterpretation, + QueryParameterBindingsImpl parameterBindings) { + final SqmTranslatorFactory sqmTranslatorFactory = sessionFactory.getQueryEngine().getSqmTranslatorFactory(); + return sqmTranslatorFactory.createSelectTranslator( + (SqmSelectStatement) hqlInterpretation.getSqmStatement(), + QueryOptions.NONE, + hqlInterpretation.getDomainParameterXref(), + parameterBindings, + new LoadQueryInfluencers( sessionFactory), + sessionFactory.getSqlTranslationEngine(), + true + ); + } + + @Override + protected SqlAstTranslator createSqlAstTranslator( + SqmTranslation sqmTranslation) { + final SqlAstTranslatorFactory sqlAstTranslatorFactory = sessionFactory + .getJdbcServices() + .getJdbcEnvironment() + .getSqlAstTranslatorFactory(); + return sqlAstTranslatorFactory.buildSelectTranslator( sessionFactory, sqmTranslation.getSqlAst() ); + } + } + + private static class SqmDeleteInterpreter extends SqmInterpreter, DeleteStatement, JdbcOperationQueryDelete> { + public SqmDeleteInterpreter(String hql, SessionFactoryImplementor sessionFactory) { + super( hql, sessionFactory ); + } + + @Override + protected SqmTranslator createSqmTranslator( + HqlInterpretation hqlInterpretation, + QueryParameterBindingsImpl parameterBindings) { + final SqmTranslatorFactory sqmTranslatorFactory = sessionFactory.getQueryEngine().getSqmTranslatorFactory(); + //noinspection unchecked + return (SqmTranslator) sqmTranslatorFactory.createMutationTranslator( + (SqmDeleteStatement) hqlInterpretation.getSqmStatement(), + QueryOptions.NONE, + hqlInterpretation.getDomainParameterXref(), + parameterBindings, + new LoadQueryInfluencers(sessionFactory), + sessionFactory.getSqlTranslationEngine() + ); + } + + @Override + protected SqlAstTranslator createSqlAstTranslator(SqmTranslation sqmTranslation) { + final SqlAstTranslatorFactory sqlAstTranslatorFactory = sessionFactory + .getJdbcServices() + .getJdbcEnvironment() + .getSqlAstTranslatorFactory(); + //noinspection unchecked + return (SqlAstTranslator) sqlAstTranslatorFactory.buildMutationTranslator( + sessionFactory, + sqmTranslation.getSqlAst() + ); + } + } + + private static class SqmUpdateInterpreter extends SqmInterpreter, UpdateStatement, JdbcOperationQueryUpdate> { + public SqmUpdateInterpreter(String hql, SessionFactoryImplementor sessionFactory) { + super( hql, sessionFactory ); + } + + @Override + protected SqmTranslator createSqmTranslator( + HqlInterpretation hqlInterpretation, + QueryParameterBindingsImpl parameterBindings) { + final SqmTranslatorFactory sqmTranslatorFactory = sessionFactory.getQueryEngine().getSqmTranslatorFactory(); + //noinspection unchecked + return (SqmTranslator) sqmTranslatorFactory.createMutationTranslator( + (SqmUpdateStatement) hqlInterpretation.getSqmStatement(), + QueryOptions.NONE, + hqlInterpretation.getDomainParameterXref(), + parameterBindings, + new LoadQueryInfluencers(sessionFactory), + sessionFactory.getSqlTranslationEngine() + ); + } + + @Override + protected SqlAstTranslator createSqlAstTranslator(SqmTranslation sqmTranslation) { + final SqlAstTranslatorFactory sqlAstTranslatorFactory = sessionFactory + .getJdbcServices() + .getJdbcEnvironment() + .getSqlAstTranslatorFactory(); + //noinspection unchecked + return (SqlAstTranslator) sqlAstTranslatorFactory.buildMutationTranslator( + sessionFactory, + sqmTranslation.getSqlAst() + ); + } + } + + private static class SqmInsertInterpreter extends SqmInterpreter, InsertStatement, JdbcOperationQueryInsert> { + public SqmInsertInterpreter(String hql, SessionFactoryImplementor sessionFactory) { + super( hql, sessionFactory ); + } + + @Override + protected SqmTranslator createSqmTranslator( + HqlInterpretation hqlInterpretation, + QueryParameterBindingsImpl parameterBindings) { + final SqmTranslatorFactory sqmTranslatorFactory = sessionFactory.getQueryEngine().getSqmTranslatorFactory(); + //noinspection unchecked + return (SqmTranslator) sqmTranslatorFactory.createMutationTranslator( + (SqmInsertStatement) hqlInterpretation.getSqmStatement(), + QueryOptions.NONE, + hqlInterpretation.getDomainParameterXref(), + parameterBindings, + new LoadQueryInfluencers(sessionFactory), + sessionFactory.getSqlTranslationEngine() + ); + } + + @Override + protected SqlAstTranslator createSqlAstTranslator(SqmTranslation sqmTranslation) { + final SqlAstTranslatorFactory sqlAstTranslatorFactory = sessionFactory + .getJdbcServices() + .getJdbcEnvironment() + .getSqlAstTranslatorFactory(); + //noinspection unchecked + return (SqlAstTranslator) sqlAstTranslatorFactory.buildMutationTranslator( + sessionFactory, + sqmTranslation.getSqlAst() + ); + } + } + + private record NonCopyingHqlInterpretationImpl( + SqmStatement sqmAst, + ParameterMetadataImplementor parameterMetadata, + DomainParameterXref domainParameterXref) implements HqlInterpretation { + @Override + public SqmStatement getSqmStatement() { + return sqmAst(); + } + + @Override + public ParameterMetadataImplementor getParameterMetadata() { + return parameterMetadata(); + } + + @Override + public DomainParameterXref getDomainParameterXref() { + return domainParameterXref(); + } + + @Override + public void validateResultType(Class resultType) { + // irrelevant here + } + } + +} diff --git a/hibernate-testing/src/main/java/org/hibernate/testing/util/ast/LoadingAstHelper.java b/hibernate-testing/src/main/java/org/hibernate/testing/util/ast/LoadingAstHelper.java new file mode 100644 index 000000000000..5d7769923b00 --- /dev/null +++ b/hibernate-testing/src/main/java/org/hibernate/testing/util/ast/LoadingAstHelper.java @@ -0,0 +1,85 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright Red Hat Inc. and Hibernate Authors + */ +package org.hibernate.testing.util.ast; + +import org.hibernate.LockOptions; +import org.hibernate.engine.spi.LoadQueryInfluencers; +import org.hibernate.engine.spi.SessionFactoryImplementor; +import org.hibernate.loader.ast.internal.LoaderSelectBuilder; +import org.hibernate.metamodel.mapping.EntityIdentifierMapping; +import org.hibernate.metamodel.mapping.EntityMappingType; +import org.hibernate.query.spi.QueryOptions; +import org.hibernate.sql.ast.tree.expression.JdbcParameter; +import org.hibernate.sql.ast.tree.select.SelectStatement; +import org.hibernate.sql.exec.internal.JdbcParameterBindingImpl; +import org.hibernate.sql.exec.internal.JdbcParameterBindingsImpl; +import org.hibernate.sql.exec.spi.JdbcParameterBindings; + +import java.util.ArrayList; +import java.util.List; + +/** + * @author Steve Ebersole + */ +public class LoadingAstHelper { + /// Translation details for loading + /// + /// @param sql The corresponding SQL + /// @param sqlAst The corresponding SQL AST + /// @param jdbcParameters The corresponding JDBC parameters + public record LoaderTranslation( + String sql, + SelectStatement sqlAst, + List jdbcParameters) { + } + + public static LoaderTranslation translateLoading( + EntityMappingType entityMappingType, + I id, + SessionFactoryImplementor sessionFactory) { + return translateLoading( entityMappingType, List.of(id), sessionFactory ); + } + + public static LoaderTranslation translateLoading( + EntityMappingType entityMappingType, + List ids, + SessionFactoryImplementor sessionFactory) { + var jdbcParameters = new ArrayList(); + var sqlAst = LoaderSelectBuilder.createSelect( + entityMappingType, + null, + entityMappingType.getIdentifierMapping(), + null, + ids.size(), + new LoadQueryInfluencers( sessionFactory ), + LockOptions.NONE, + jdbcParameters::add, + sessionFactory + ); + var sqlAstTranslator = sessionFactory + .getJdbcServices() + .getJdbcEnvironment() + .getSqlAstTranslatorFactory() + .buildSelectTranslator( sessionFactory, sqlAst ); + var jdbcOperation = sqlAstTranslator.translate( + buildJdbcParameterBindings( entityMappingType.getIdentifierMapping(), ids, jdbcParameters ), + QueryOptions.NONE + ); + return new LoaderTranslation( jdbcOperation.getSqlString(), sqlAst, jdbcParameters ); + } + + private static JdbcParameterBindings buildJdbcParameterBindings( + EntityIdentifierMapping identifierMapping, + List ids, + ArrayList jdbcParameters) { + final JdbcParameterBindings jdbcParameterBindings = new JdbcParameterBindingsImpl( jdbcParameters.size() ); + identifierMapping.forEachJdbcType( (position, jdbcMapping) -> jdbcParameterBindings.addBinding( + jdbcParameters.get( position ), + new JdbcParameterBindingImpl( jdbcMapping, null ) + ) ); + return jdbcParameterBindings; + } + +} From 8c97acd08d17b5f1aee70e1038c66e4f5c1cff07 Mon Sep 17 00:00:00 2001 From: Steve Ebersole Date: Thu, 13 Nov 2025 16:08:58 -0700 Subject: [PATCH 2/3] HHH-19925 - Locking root(s) should be based on select-clause, not from-clause --- .../sqm/sql/BaseSqmToSqlAstConverter.java | 42 ++++++++++++- .../LockingBasedOnSelectClauseTests.java | 63 +++++++++++++++++-- 2 files changed, 97 insertions(+), 8 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java index 7fdb293d83cd..5bcaab208492 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java +++ b/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) { diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/locking/LockingBasedOnSelectClauseTests.java b/hibernate-core/src/test/java/org/hibernate/orm/test/locking/LockingBasedOnSelectClauseTests.java index afff6d8bc6b0..1a198640aa27 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/locking/LockingBasedOnSelectClauseTests.java +++ b/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,7 +162,7 @@ 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") @@ -117,7 +170,7 @@ void testLoadingTranslation(SessionFactoryScope factoryScope) { 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; @@ -125,9 +178,9 @@ public static class Book { 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; } } From d3b777fae6c82227c3fe7f791f58ab31bc64f095 Mon Sep 17 00:00:00 2001 From: Steve Ebersole Date: Fri, 14 Nov 2025 07:51:26 -0700 Subject: [PATCH 3/3] HHH-19925 - Locking root(s) should be based on select-clause, not from-clause --- .../community/dialect/DerbyDialect.java | 7 +- .../community/dialect/DerbyLegacyDialect.java | 7 +- .../dialect/DerbyLockingClauseStrategy.java | 8 +- .../community/dialect/TeradataDialect.java | 7 +- .../java/org/hibernate/dialect/Dialect.java | 8 +- .../internal/SqlAstBasedLockingStrategy.java | 7 +- .../ast/internal/AbstractNaturalIdLoader.java | 2 +- .../internal/DatabaseSnapshotExecutor.java | 2 +- .../ast/internal/LoaderSelectBuilder.java | 16 ++-- .../internal/MatchingIdSelectionHelper.java | 2 +- .../cte/AbstractCteMutationHandler.java | 2 +- .../internal/cte/CteInsertHandler.java | 2 +- .../temptable/TableBasedInsertHandler.java | 3 +- .../sqm/sql/BaseSqmToSqlAstConverter.java | 21 +++-- .../StandardLockingClauseStrategy.java | 79 ++++++++++++++----- .../sql/ast/spi/AbstractSqlAstTranslator.java | 8 +- .../sql/ast/tree/select/QuerySpec.java | 20 +++++ .../sql/ast/tree/select/SelectStatement.java | 19 +---- .../sql/exec/internal/lock/TableLock.java | 3 +- .../LockingBasedOnSelectClauseTests.java | 60 +++++++------- 20 files changed, 171 insertions(+), 112 deletions(-) diff --git a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/DerbyDialect.java b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/DerbyDialect.java index f6984901261e..5cb7bc92015d 100644 --- a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/DerbyDialect.java +++ b/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 rootPathsForLocking) { + return new DerbyLockingClauseStrategy( this, lockKind, rowLockStrategy, lockOptions, rootPathsForLocking ); } @Override diff --git a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/DerbyLegacyDialect.java b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/DerbyLegacyDialect.java index 3aefecb45dae..38c77c1a2b65 100644 --- a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/DerbyLegacyDialect.java +++ b/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 rootPathsForLocking) { + return new DerbyLockingClauseStrategy( this, lockKind, rowLockStrategy, lockOptions, rootPathsForLocking ); } @Override diff --git a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/DerbyLockingClauseStrategy.java b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/DerbyLockingClauseStrategy.java index 261793a9c936..9a44dd6eabd4 100644 --- a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/DerbyLockingClauseStrategy.java +++ b/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 rootPathsForLocking) { + super( dialect, lockKind, rowLockStrategy, lockOptions, rootPathsForLocking ); } @Override diff --git a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/TeradataDialect.java b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/TeradataDialect.java index 1f836e863951..2e7dfda78dff 100644 --- a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/TeradataDialect.java +++ b/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 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 diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java b/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java index 1fd8ed286548..7601ab4ce695 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java +++ b/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 rootPathsForLocking) { + return new StandardLockingClauseStrategy( this, lockKind, rowLockStrategy, lockOptions, rootPathsForLocking ); } /** diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/lock/internal/SqlAstBasedLockingStrategy.java b/hibernate-core/src/main/java/org/hibernate/dialect/lock/internal/SqlAstBasedLockingStrategy.java index 6c483c05c25b..2c018ee7decc 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/lock/internal/SqlAstBasedLockingStrategy.java +++ b/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... @@ -154,7 +157,7 @@ public void lock( ); } - final var selectStatement = new SelectStatement( rootQuerySpec, List.of( idResult ), List.of( entityPath ) ); + final var selectStatement = new SelectStatement( rootQuerySpec, List.of( idResult ) ); final JdbcSelect selectOperation = session.getDialect().getSqlAstTranslatorFactory() .buildSelectTranslator( factory, selectStatement ) diff --git a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/AbstractNaturalIdLoader.java b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/AbstractNaturalIdLoader.java index 4fb3414e6548..b8e5529868fa 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/AbstractNaturalIdLoader.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/AbstractNaturalIdLoader.java @@ -215,7 +215,7 @@ public Object resolveNaturalIdToId(Object naturalIdValue, SharedSessionContractI return executeNaturalIdQuery( naturalIdValue, LockOptions.NONE, - new SelectStatement( rootQuerySpec, singletonList( domainResult ), List.of() ), + new SelectStatement( rootQuerySpec, singletonList( domainResult ) ), rootTableGroup, rootQuerySpec::applyPredicate, sqlAstCreationState, diff --git a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/DatabaseSnapshotExecutor.java b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/DatabaseSnapshotExecutor.java index c6dd25b1c55e..7571ee6b0e76 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/DatabaseSnapshotExecutor.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/DatabaseSnapshotExecutor.java @@ -128,7 +128,7 @@ class DatabaseSnapshotExecutor { } ); - final var selectStatement = new SelectStatement( rootQuerySpec, domainResults, List.of() ); + final var selectStatement = new SelectStatement( rootQuerySpec, domainResults ); jdbcSelect = sessionFactory.getJdbcServices().getJdbcEnvironment().getSqlAstTranslatorFactory() .buildSelectTranslator( sessionFactory, selectStatement ) diff --git a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/LoaderSelectBuilder.java b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/LoaderSelectBuilder.java index de1a1a0fcd0c..3b84bb7eb160 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/LoaderSelectBuilder.java +++ b/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 ); @@ -177,7 +179,7 @@ public static SelectStatement createSelectBySingleArrayParameter( builder.applyFiltering( rootQuerySpec, rootTableGroup, (Restrictable) loadable, sqlAstCreationState ); } - return new SelectStatement( rootQuerySpec, domainResults, List.of( rootNavigablePath ) ); + return new SelectStatement( rootQuerySpec, domainResults ); } private static void applyArrayParamRestriction( @@ -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 ); @@ -505,7 +509,7 @@ else if ( cachedDomainResult != null ) { applyFiltering( rootQuerySpec, rootTableGroup, (Restrictable) loadable, sqlAstCreationState ); } - return new SelectStatement( rootQuerySpec, domainResults, List.of( rootNavigablePath ) ); + return new SelectStatement( rootQuerySpec, domainResults ); } private List> buildRequestedDomainResults( @@ -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() ); @@ -1024,8 +1027,7 @@ private SelectStatement generateSelect(SubselectFetch subselect) { rootTableGroup, sqlAstCreationState ) - ), - singletonList( rootNavigablePath ) + ) ); } diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/MatchingIdSelectionHelper.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/MatchingIdSelectionHelper.java index 674fea3e50dc..9d66ce48869e 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/MatchingIdSelectionHelper.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/MatchingIdSelectionHelper.java @@ -124,7 +124,7 @@ public static SelectStatement generateMatchingIdSelectStatement( sqmConverter ); - return new SelectStatement( idSelectionQuery, domainResults, List.of() ); + return new SelectStatement( idSelectionQuery, domainResults ); } /// Generates a query-spec for selecting all ids matching the restriction defined as part diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/cte/AbstractCteMutationHandler.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/cte/AbstractCteMutationHandler.java index e3b46a2365ee..d36d4c9b1892 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/cte/AbstractCteMutationHandler.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/cte/AbstractCteMutationHandler.java @@ -138,7 +138,7 @@ public AbstractCteMutationHandler( // Create the main query spec that will return the count of final QuerySpec querySpec = new QuerySpec( true, 1 ); final List> domainResults = new ArrayList<>( 1 ); - final SelectStatement statement = new SelectStatement( querySpec, domainResults, List.of() ); + final SelectStatement statement = new SelectStatement( querySpec, domainResults ); final JdbcServices jdbcServices = factory.getJdbcServices(); final SqlAstTranslator translator = jdbcServices.getJdbcEnvironment() .getSqlAstTranslatorFactory() diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/cte/CteInsertHandler.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/cte/CteInsertHandler.java index 307d9cb55b4f..ffe734e4e842 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/cte/CteInsertHandler.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/cte/CteInsertHandler.java @@ -291,7 +291,7 @@ public CteInsertHandler( // Create the main query spec that will return the count of rows final QuerySpec querySpec = new QuerySpec( true, 1 ); final List> domainResults = new ArrayList<>( 1 ); - final SelectStatement statement = new SelectStatement( querySpec, domainResults, List.of() ); + final SelectStatement statement = new SelectStatement( querySpec, domainResults ); final CteStatement entityCte; if ( additionalInsertValues.requiresRowNumberIntermediate() ) { diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/temptable/TableBasedInsertHandler.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/temptable/TableBasedInsertHandler.java index 05b1fc631c52..77dd84d9e8f5 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/temptable/TableBasedInsertHandler.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/temptable/TableBasedInsertHandler.java @@ -593,8 +593,7 @@ private RootTableInserter createRootTableInserter( null, false ) - ), - List.of() + ) ); temporaryTableIdentitySelect = jdbcServices.getJdbcEnvironment() .getSqlAstTranslatorFactory() diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java index 5bcaab208492..3583ad25c141 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java @@ -1640,12 +1640,12 @@ public SelectStatement visitSelectStatement(SqmSelectStatement statement) { final QueryPart queryPart = visitQueryPart( statement.getQueryPart() ); final List> domainResults = queryPart.isRoot() ? this.domainResults : emptyList(); try { - return new SelectStatement( cteContainer, queryPart, domainResults, rootPathsForLocking ); + return new SelectStatement( cteContainer, queryPart, domainResults ); } finally { this.currentSqmStatement = oldSqmStatement; this.cteContainer = oldCteContainer; - rootPathsForLocking = null; + rootPathsForLockingCollector = null; } } @@ -1754,7 +1754,7 @@ public CteStatement visitCteStatement(SqmCteStatement sqmCteStatement) { final CteStatement cteStatement = new CteStatement( cteTable, - new SelectStatement( subCteContainer, group, emptyList(), emptyList() ), + new SelectStatement( subCteContainer, group, emptyList() ), sqmCteStatement.getMaterialization(), sqmCteStatement.getSearchClauseKind(), visitSearchBySpecifications( cteTable, sqmCteStatement.getSearchBySpecifications() ), @@ -2206,7 +2206,6 @@ private TableGroup findTableGroupByPath(NavigablePath navigablePath) { return getFromClauseAccess().getTableGroup( navigablePath ); } - private List rootPathsForLocking; private Consumer rootPathsForLockingCollector; @Override @@ -2214,13 +2213,11 @@ public SelectClause visitSelectClause(SqmSelectClause selectClause) { currentClauseStack.push( Clause.SELECT ); try { final SelectClause sqlSelectClause = currentQuerySpec().getSelectClause(); - if ( sqmQueryPartStack.depth() == 1 ) { - rootPathsForLockingCollector = (navigablePath) -> { - if ( rootPathsForLocking == null ) { - rootPathsForLocking = new ArrayList<>(); - } - rootPathsForLocking.add( navigablePath ); - }; + if ( sqmQueryPartStack.depth() == 1 && currentClauseStack.depth() == 1 ) { + // these 2 conditions combined *should* indicate we have the + // root query-spec of a top-level select statement + rootPathsForLockingCollector = (path) -> + currentQuerySpec().applyRootPathForLocking( path ); } if ( selectClause == null ) { final SqmFrom implicitSelection = determineImplicitSelection( (SqmQuerySpec) getCurrentSqmQueryPart() ); @@ -7190,7 +7187,7 @@ public SelectStatement visitSubQueryExpression(SqmSubQuery sqmSubQuery) { final QueryPart queryPart = visitQueryPart( sqmSubQuery.getQueryPart() ); currentlyProcessingJoin = oldJoin; this.cteContainer = oldCteContainer; - return new SelectStatement( cteContainer, queryPart, emptyList(), emptyList() ); + return new SelectStatement( cteContainer, queryPart, emptyList() ); } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/sql/ast/internal/StandardLockingClauseStrategy.java b/hibernate-core/src/main/java/org/hibernate/sql/ast/internal/StandardLockingClauseStrategy.java index e73c6b35db02..2578893422d4 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/ast/internal/StandardLockingClauseStrategy.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/ast/internal/StandardLockingClauseStrategy.java @@ -20,6 +20,7 @@ import org.hibernate.metamodel.mapping.internal.BasicValuedCollectionPart; import org.hibernate.persister.entity.EntityPersister; import org.hibernate.persister.entity.mutation.EntityTableMapping; +import org.hibernate.spi.NavigablePath; import org.hibernate.sql.ast.SqlAstJoinType; import org.hibernate.sql.ast.spi.LockingClauseStrategy; import org.hibernate.sql.ast.spi.SqlAppender; @@ -60,14 +61,18 @@ public class StandardLockingClauseStrategy implements LockingClauseStrategy { */ private boolean queryHasOuterJoins = false; + private final Set rootsForLocking; + private Set rootsToLock; private Set joinsToLock; + private Set pathsToLock; public StandardLockingClauseStrategy( Dialect dialect, PessimisticLockKind lockKind, RowLockStrategy rowLockStrategy, - LockOptions lockOptions) { + LockOptions lockOptions, + Set rootsForLocking) { // NOTE: previous versions would limit collection based on RowLockStrategy. // however, this causes problems with the new follow-on locking approach @@ -78,6 +83,8 @@ public StandardLockingClauseStrategy( this.lockKind = lockKind; this.lockingScope = lockOptions.getScope(); this.timeout = lockOptions.getTimeout(); + + this.rootsForLocking = rootsForLocking == null ? Set.of() : rootsForLocking; } @Override @@ -89,36 +96,64 @@ public void registerRoot(TableGroup root) { } } - if ( rootsToLock == null ) { - rootsToLock = new HashSet<>(); + if ( rootsForLocking.contains( root.getNavigablePath() ) ) { + if ( rootsToLock == null ) { + rootsToLock = new HashSet<>(); + } + if ( pathsToLock == null ) { + pathsToLock = new HashSet<>(); + } + rootsToLock.add( root ); + pathsToLock.add( root.getNavigablePath() ); } - rootsToLock.add( root ); } @Override public void registerJoin(TableGroupJoin join) { checkForOuterJoins( join ); - if ( lockingScope == Locking.Scope.INCLUDE_COLLECTIONS ) { - // if the TableGroup is an owned (aka, non-inverse) collection, - // and we are to lock collections, track it - if ( join.getJoinedGroup().getModelPart() instanceof PluralAttributeMapping attrMapping ) { - if ( !attrMapping.getCollectionDescriptor().isInverse() ) { - // owned collection - if ( attrMapping.getElementDescriptor() instanceof BasicValuedCollectionPart ) { - // an element-collection - trackJoin( join ); + // we only want to consider applying locks to joins in 2 cases: + // 1) It is a root path for locking (aka occurs in the domain select-clause) + // 2) It's left-hand side is to be locked + if ( isRootForLocking( join ) ) { + trackJoin( join ); + } + else if ( isLhsLocked( join ) ) { + if ( lockingScope == Locking.Scope.INCLUDE_COLLECTIONS ) { + // if the TableGroup is an owned (aka, non-inverse) collection, + // and we are to lock collections, track it + if ( join.getJoinedGroup().getModelPart() instanceof PluralAttributeMapping attrMapping ) { + if ( !attrMapping.getCollectionDescriptor().isInverse() ) { + // owned collection + if ( attrMapping.getElementDescriptor() instanceof BasicValuedCollectionPart ) { + // an element-collection + trackJoin( join ); + } } } } - } - else if ( lockingScope == Locking.Scope.INCLUDE_FETCHES ) { - if ( join.getJoinedGroup().isFetched() ) { - trackJoin( join ); + else if ( lockingScope == Locking.Scope.INCLUDE_FETCHES ) { + if ( join.getJoinedGroup().isFetched() ) { + trackJoin( join ); + } } } } + private boolean isRootForLocking(TableGroupJoin join) { + return rootsForLocking.contains( join.getNavigablePath() ); + } + + private boolean isLhsLocked(TableGroupJoin join) { + // TODO (pessimistic-locking) : The use of NavigablePath#parent for LHS here is not ideal. + // However, the only alternative is to change the method signature to pass the + // join's LHS which would have a broad impact on Dialects and translators. + // I'm sure this will miss some cases, but let's start here fow now and deal with + // these other cases as they come up. + return pathsToLock != null + && pathsToLock.contains( join.getNavigablePath().getParent() ); + } + private void checkForOuterJoins(TableGroupJoin join) { if ( queryHasOuterJoins ) { // perf out @@ -148,7 +183,11 @@ private void trackJoin(TableGroupJoin join) { if ( joinsToLock == null ) { joinsToLock = new LinkedHashSet<>(); } + if ( pathsToLock == null ) { + pathsToLock = new HashSet<>(); + } joinsToLock.add( join ); + pathsToLock.add( join.getNavigablePath() ); } @Override @@ -194,8 +233,10 @@ private String collectLockItems() { } final List lockItems = new ArrayList<>(); - for ( TableGroup root : rootsToLock ) { - collectLockItems( root, lockItems ); + if ( rootsToLock != null ) { + for ( TableGroup root : rootsToLock ) { + collectLockItems( root, lockItems ); + } } if ( joinsToLock != null ) { for ( TableGroupJoin join : joinsToLock ) { diff --git a/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/AbstractSqlAstTranslator.java b/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/AbstractSqlAstTranslator.java index 2deaa363d3b1..2d3d3a645447 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/AbstractSqlAstTranslator.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/AbstractSqlAstTranslator.java @@ -6492,7 +6492,6 @@ protected Predicate determineLateralEmulationPredicate(TableGroup tableGroup) { new SelectStatement( statement, new QueryGroup( false, SetOperator.INTERSECT, queryParts ), - emptyList(), emptyList() ), false, @@ -6541,7 +6540,7 @@ protected Predicate determineLateralEmulationPredicate(TableGroup tableGroup) { ); return new ExistsPredicate( - new SelectStatement( statement, existsQuery, emptyList(), emptyList() ), + new SelectStatement( statement, existsQuery, emptyList() ), false, booleanType ); @@ -6584,7 +6583,7 @@ protected Predicate determineLateralEmulationPredicate(TableGroup tableGroup) { ); final ExistsPredicate existsPredicate = new ExistsPredicate( - new SelectStatement( statement, existsQuery, emptyList(), emptyList() ), + new SelectStatement( statement, existsQuery, emptyList() ), false, booleanType ); @@ -6702,7 +6701,7 @@ protected Predicate determineLateralEmulationPredicate(TableGroup tableGroup) { List.of( existsPredicate, new BetweenPredicate( - new SelectStatement( statement, countQuery, emptyList(), emptyList() ), + new SelectStatement( statement, countQuery, emptyList() ), countLower, countUpper, false, @@ -6765,7 +6764,6 @@ private SelectStatement stripToSelectClause(SelectStatement statement) { return new SelectStatement( statement, stripToSelectClause( statement.getQueryPart() ), - emptyList(), emptyList() ); } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/select/QuerySpec.java b/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/select/QuerySpec.java index c3d79bf186b7..7dabae246724 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/select/QuerySpec.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/select/QuerySpec.java @@ -5,10 +5,13 @@ package org.hibernate.sql.ast.tree.select; import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Set; import java.util.function.Consumer; import java.util.function.Function; +import org.hibernate.spi.NavigablePath; import org.hibernate.sql.ast.SqlAstWalker; import org.hibernate.sql.ast.spi.SqlAstTreeHelper; import org.hibernate.sql.ast.tree.SqlAstNode; @@ -30,6 +33,8 @@ public class QuerySpec extends QueryPart implements SqlAstNode, PredicateContain private List groupByClauseExpressions = Collections.emptyList(); private Predicate havingClauseRestrictions; + private Set rootPathsForLocking; + public QuerySpec(boolean isRoot) { super( isRoot ); this.fromClause = new FromClause(); @@ -87,6 +92,21 @@ public SelectClause getSelectClause() { return selectClause; } + /// Set of [NavigablePath] references to be considered roots + /// for locking purposes. + public Set getRootPathsForLocking() { + return rootPathsForLocking; + } + + /// Applies a [NavigablePath] to be considered a root for the + /// purpose of potential locking. + public void applyRootPathForLocking(NavigablePath path) { + if ( rootPathsForLocking == null ) { + rootPathsForLocking = new HashSet<>(); + } + rootPathsForLocking.add( path ); + } + public Predicate getWhereClauseRestrictions() { return whereClauseRestrictions; } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/select/SelectStatement.java b/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/select/SelectStatement.java index 1f896070831b..29f8438a8f9c 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/select/SelectStatement.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/select/SelectStatement.java @@ -7,7 +7,6 @@ import org.hibernate.metamodel.mapping.JdbcMapping; import org.hibernate.metamodel.mapping.JdbcMappingContainer; import org.hibernate.query.sqm.sql.internal.DomainResultProducer; -import org.hibernate.spi.NavigablePath; import org.hibernate.sql.ast.SqlAstWalker; import org.hibernate.sql.ast.spi.SqlExpressionResolver; import org.hibernate.sql.ast.spi.SqlSelection; @@ -29,28 +28,24 @@ public class SelectStatement extends AbstractStatement implements SqlAstNode, Expression, DomainResultProducer { private final QueryPart queryPart; private final List> domainResults; - private final List rootPathsForLocking; public SelectStatement(QueryPart queryPart) { - this( queryPart, Collections.emptyList(), Collections.emptyList() ); + this( queryPart, Collections.emptyList() ); } public SelectStatement( QueryPart queryPart, - List> domainResults, - List rootPathsForLocking) { - this( null, queryPart, domainResults, rootPathsForLocking ); + List> domainResults) { + this( null, queryPart, domainResults ); } public SelectStatement( CteContainer cteContainer, QueryPart queryPart, - List> domainResults, - List rootPathsForLocking) { + List> domainResults) { super( cteContainer ); this.queryPart = queryPart; this.domainResults = domainResults; - this.rootPathsForLocking = rootPathsForLocking == null ? Collections.emptyList() : rootPathsForLocking; } @Override @@ -70,12 +65,6 @@ public List> getDomainResultDescriptors() { return domainResults; } - /// List of [NavigablePath] references to be considered roots - /// for locking purposes. - public List getRootPathsForLocking() { - return rootPathsForLocking; - } - @Override public void accept(SqlAstWalker walker) { walker.visitSelectStatement( this ); diff --git a/hibernate-core/src/main/java/org/hibernate/sql/exec/internal/lock/TableLock.java b/hibernate-core/src/main/java/org/hibernate/sql/exec/internal/lock/TableLock.java index ee05802d6f08..a7914aa936db 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/exec/internal/lock/TableLock.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/exec/internal/lock/TableLock.java @@ -122,6 +122,7 @@ public TableLock( } querySpec.getFromClause().addRoot( physicalTableGroup ); + querySpec.applyRootPathForLocking( rootPath ); creationStates = new LockingCreationStates( querySpec, @@ -238,7 +239,7 @@ private void applyCompositeKeyRestriction(List entityKeys, SharedSess public void performActions(Map entityDetailsMap, QueryOptions lockingQueryOptions, SharedSessionContractImplementor session) { final var sessionFactory = session.getSessionFactory(); final var jdbcServices = sessionFactory.getJdbcServices(); - final var selectStatement = new SelectStatement( querySpec, domainResults, List.of( rootPath ) ); + final var selectStatement = new SelectStatement( querySpec, domainResults ); final List results = jdbcServices.getJdbcSelectExecutor() .executeQuery( diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/locking/LockingBasedOnSelectClauseTests.java b/hibernate-core/src/test/java/org/hibernate/orm/test/locking/LockingBasedOnSelectClauseTests.java index 1a198640aa27..707cf5def72a 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/locking/LockingBasedOnSelectClauseTests.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/locking/LockingBasedOnSelectClauseTests.java @@ -11,18 +11,22 @@ import jakarta.persistence.LockModeType; import jakarta.persistence.ManyToOne; import jakarta.persistence.Table; +import org.hibernate.spi.NavigablePath; import org.hibernate.sql.ast.tree.Statement; import org.hibernate.sql.ast.tree.select.SelectStatement; import org.hibernate.testing.orm.junit.DomainModel; import org.hibernate.testing.orm.junit.JiraKey; import org.hibernate.testing.orm.junit.SessionFactory; import org.hibernate.testing.orm.junit.SessionFactoryScope; +import org.hibernate.testing.orm.transaction.TransactionUtil; import org.hibernate.testing.util.ast.HqlHelper; import org.hibernate.testing.util.ast.LoadingAstHelper; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import java.util.Iterator; + import static org.assertj.core.api.Assertions.assertThat; /** @@ -57,23 +61,15 @@ void testBasicHqlUsage(SessionFactoryScope factoryScope) { session.createQuery( "select b.author from Book b" ) .setLockMode( LockModeType.PESSIMISTIC_WRITE ) .list(); - // NOTE : The correct outcome here is for the authors table to be locked as the root selection. - // However, that is not what happens today - atm, the books table is locked (as the root - // of the from-clause). - // So ideally we want to make sure that - - // * authors is locked - // * books is not - // Unfortunately we cannot even craft a good assertion here because some of the dialects - // will lock both. - -// TransactionUtil.assertRowLock( -// factoryScope, -// "authors", -// "name", -// "id", -// 1, -// session.getDialect().getLockingSupport().getMetadata().getWriteRowLockStrategy() == RowLockStrategy.NONE -// ); + // The correct outcome here is for the authors table to be locked as the root selection. + TransactionUtil.assertRowLock( + factoryScope, + "authors", + "name", + "id", + 1, + true + ); } ); } @@ -98,8 +94,8 @@ void testBasicHqlTranslation(SessionFactoryScope factoryScope) { 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() ) + assertThat( selectAst.getQuerySpec().getRootPathsForLocking() ).hasSize( 1 ); + assertThat( selectAst.getQuerySpec().getRootPathsForLocking().iterator().next().getFullPath() ) .isEqualTo( BOOK_AUTHOR_PATH_HQL ); } @@ -113,8 +109,8 @@ void testScalarHqlTranslation(SessionFactoryScope factoryScope) { 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() ) + assertThat( selectAst.getQuerySpec().getRootPathsForLocking() ).hasSize( 1 ); + assertThat( selectAst.getQuerySpec().getRootPathsForLocking().iterator().next().getFullPath() ) .isEqualTo( BOOK_PATH_HQL ); } @@ -128,11 +124,10 @@ void testScalarHqlTranslation2(SessionFactoryScope factoryScope) { 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 ); + assertThat( selectAst.getQuerySpec().getRootPathsForLocking() ).hasSize( 2 ); + final Iterator paths = selectAst.getQuerySpec().getRootPathsForLocking().iterator(); + assertThat( paths.next().getFullPath() ).isEqualTo( BOOK_PATH_HQL ); + assertThat( paths.next().getFullPath() ).isEqualTo( BOOK_AUTHOR_PATH_HQL ); } @Test @@ -145,11 +140,10 @@ void testDynamicInstantiationHqlTranslation(SessionFactoryScope factoryScope) { 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 ); + assertThat( selectAst.getQuerySpec().getRootPathsForLocking() ).hasSize( 2 ); + final Iterator paths = selectAst.getQuerySpec().getRootPathsForLocking().iterator(); + assertThat( paths.next().getFullPath() ).isEqualTo( BOOK_PATH_HQL ); + assertThat( paths.next().getFullPath() ).isEqualTo( BOOK_AUTHOR_PATH_HQL ); } @Test @@ -160,8 +154,8 @@ void testLoadingTranslation(SessionFactoryScope factoryScope) { 1, factoryScope.getSessionFactory() ); - assertThat( translation.sqlAst().getRootPathsForLocking() ).hasSize( 1 ); - assertThat( translation.sqlAst().getRootPathsForLocking().get( 0 ).getFullPath() ) + assertThat( translation.sqlAst().getQuerySpec().getRootPathsForLocking() ).hasSize( 1 ); + assertThat( translation.sqlAst().getQuerySpec().getRootPathsForLocking().iterator().next().getFullPath() ) .isEqualTo( BOOK_PATH ); }