From a8c87cd284b4328cbd458f9f2ea498f4729b780c Mon Sep 17 00:00:00 2001 From: Christian Beikov Date: Mon, 15 May 2023 11:23:04 +0200 Subject: [PATCH] HHH-16541 Don't consider uninitialized LazyTableGroup for follow-on locking emulation. Fix lock mode upgrade for follow-on locking --- .../dialect/SybaseASELegacyDialect.java | 31 ++------ .../SybaseASELegacySqlAstTranslator.java | 71 ++++++++++++++--- .../hibernate/dialect/SybaseASEDialect.java | 27 ++----- .../dialect/SybaseASESqlAstTranslator.java | 69 +++++++++++++++- .../org/hibernate/query/MutationQuery.java | 20 ++++- .../org/hibernate/query/SelectionQuery.java | 3 + .../query/spi/AbstractSelectionQuery.java | 6 ++ .../internal/MatchingIdSelectionHelper.java | 2 +- .../ExecuteWithTemporaryTableHelper.java | 2 +- .../result/internal/OutputsImpl.java | 3 +- .../sql/ast/spi/AbstractSqlAstTranslator.java | 4 +- .../sql/ast/tree/from/TableGroupJoin.java | 5 ++ .../sql/ast/tree/from/TableJoin.java | 1 + .../sql/ast/tree/from/TableReferenceJoin.java | 5 ++ .../JdbcSelectExecutorStandardImpl.java | 3 +- .../entity/AbstractEntityInitializer.java | 13 ++++ ...luesSourceProcessingStateStandardImpl.java | 78 ++++++++++++------- .../spi/JdbcValuesSourceProcessingState.java | 4 + .../test/locking/jpa/FollowOnLockingTest.java | 10 ++- .../mutation/multitable/IdSelectionTests.java | 2 +- 20 files changed, 258 insertions(+), 101 deletions(-) diff --git a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/SybaseASELegacyDialect.java b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/SybaseASELegacyDialect.java index 90b293268f..7fd7e0feeb 100644 --- a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/SybaseASELegacyDialect.java +++ b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/SybaseASELegacyDialect.java @@ -12,6 +12,7 @@ import java.sql.SQLException; import java.sql.Types; import java.util.Map; +import org.hibernate.LockMode; import org.hibernate.LockOptions; import org.hibernate.boot.model.TypeContributions; import org.hibernate.dialect.DatabaseVersion; @@ -590,34 +591,16 @@ public class SybaseASELegacyDialect extends SybaseLegacyDialect { } @Override - public RowLockStrategy getWriteRowLockStrategy() { - return getVersion().isSameOrAfter( 15, 7 ) ? RowLockStrategy.COLUMN : RowLockStrategy.TABLE; - } - - @Override - public String getForUpdateString() { - return getVersion().isBefore( 15, 7 ) ? "" : " for update"; - } - - @Override - public String getForUpdateString(String aliases) { - return getVersion().isBefore( 15, 7 ) - ? "" - : getForUpdateString() + " of " + aliases; + public boolean supportsSkipLocked() { + return true; } @Override public String appendLockHint(LockOptions mode, String tableName) { - //TODO: is this really necessary??! - return getVersion().isBefore( 15, 7 ) ? super.appendLockHint( mode, tableName ) : tableName; - } - - @Override - public String applyLocksToSql(String sql, LockOptions aliasedLockOptions, Map keyColumnNames) { - //TODO: is this really correct? - return getVersion().isBefore( 15, 7 ) - ? super.applyLocksToSql( sql, aliasedLockOptions, keyColumnNames ) - : sql + new ForUpdateFragment( this, aliasedLockOptions, keyColumnNames ).toFragmentString(); + final String lockHint = super.appendLockHint( mode, tableName ); + return mode.getLockMode() == LockMode.UPGRADE_SKIPLOCKED || mode.getTimeOut() == LockOptions.SKIP_LOCKED + ? lockHint + " readpast" + : lockHint; } @Override diff --git a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/SybaseASELegacySqlAstTranslator.java b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/SybaseASELegacySqlAstTranslator.java index 6cd8e2c98e..947b09832f 100644 --- a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/SybaseASELegacySqlAstTranslator.java +++ b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/SybaseASELegacySqlAstTranslator.java @@ -10,8 +10,10 @@ import java.util.List; import java.util.function.Consumer; import org.hibernate.LockMode; +import org.hibernate.LockOptions; import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.query.sqm.ComparisonOperator; +import org.hibernate.sql.ast.Clause; import org.hibernate.sql.ast.SqlAstJoinType; import org.hibernate.sql.ast.SqlAstNodeRenderingMode; import org.hibernate.sql.ast.spi.AbstractSqlAstTranslator; @@ -46,6 +48,8 @@ import org.hibernate.sql.exec.spi.JdbcOperation; */ public class SybaseASELegacySqlAstTranslator extends AbstractSqlAstTranslator { + private static final String UNION_ALL = " union all "; + public SybaseASELegacySqlAstTranslator(SessionFactoryImplementor sessionFactory, Statement statement) { super( sessionFactory, statement ); } @@ -109,14 +113,64 @@ public class SybaseASELegacySqlAstTranslator extends Ab @Override protected boolean renderNamedTableReference(NamedTableReference tableReference, LockMode lockMode) { - super.renderNamedTableReference( tableReference, lockMode ); - if ( getDialect().getVersion().isBefore( 15, 7 ) ) { - if ( LockMode.READ.lessThan( lockMode ) ) { - appendSql( " holdlock" ); + final String tableExpression = tableReference.getTableExpression(); + if ( tableReference instanceof UnionTableReference && lockMode != LockMode.NONE && tableExpression.charAt( 0 ) == '(' ) { + // SQL Server requires to push down the lock hint to the actual table names + int searchIndex = 0; + int unionIndex; + while ( ( unionIndex = tableExpression.indexOf( UNION_ALL, searchIndex ) ) != -1 ) { + append( tableExpression, searchIndex, unionIndex ); + renderLockHint( lockMode ); + appendSql( UNION_ALL ); + searchIndex = unionIndex + UNION_ALL.length(); + } + append( tableExpression, searchIndex, tableExpression.length() - 2 ); + renderLockHint( lockMode ); + appendSql( " )" ); + + registerAffectedTable( tableReference ); + final Clause currentClause = getClauseStack().getCurrent(); + if ( rendersTableReferenceAlias( currentClause ) ) { + final String identificationVariable = tableReference.getIdentificationVariable(); + if ( identificationVariable != null ) { + appendSql( ' ' ); + appendSql( identificationVariable ); + } + } + } + else { + super.renderNamedTableReference( tableReference, lockMode ); + renderLockHint( lockMode ); + } + // Just always return true because SQL Server doesn't support the FOR UPDATE clause + return true; + } + + private void renderLockHint(LockMode lockMode) { + final int effectiveLockTimeout = getEffectiveLockTimeout( lockMode ); + switch ( lockMode ) { + case PESSIMISTIC_READ: + case PESSIMISTIC_WRITE: + case WRITE: { + switch ( effectiveLockTimeout ) { + case LockOptions.SKIP_LOCKED: + appendSql( " holdlock readpast" ); + break; + default: + appendSql( " holdlock" ); + break; + } + break; + } + case UPGRADE_SKIPLOCKED: { + appendSql( " holdlock readpast" ); + break; + } + case UPGRADE_NOWAIT: { + appendSql( " holdlock" ); + break; } - return true; } - return false; } @Override @@ -152,10 +206,7 @@ public class SybaseASELegacySqlAstTranslator extends Ab @Override protected void renderForUpdateClause(QuerySpec querySpec, ForUpdateClause forUpdateClause) { - if ( getDialect().getVersion().isBefore( 15, 7 ) ) { - return; - } - super.renderForUpdateClause( querySpec, forUpdateClause ); + // Sybase ASE does not really support the FOR UPDATE clause } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/SybaseASEDialect.java b/hibernate-core/src/main/java/org/hibernate/dialect/SybaseASEDialect.java index 7261f9bc9e..69652e593d 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/SybaseASEDialect.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/SybaseASEDialect.java @@ -12,6 +12,7 @@ import java.sql.SQLException; import java.sql.Types; import java.util.Map; +import org.hibernate.LockMode; import org.hibernate.LockOptions; import org.hibernate.boot.model.TypeContributions; import org.hibernate.dialect.pagination.LimitHandler; @@ -592,30 +593,16 @@ public class SybaseASEDialect extends SybaseDialect { } @Override - public RowLockStrategy getWriteRowLockStrategy() { - return RowLockStrategy.COLUMN; - } - - @Override - public String getForUpdateString() { - return " for update"; - } - - @Override - public String getForUpdateString(String aliases) { - return getForUpdateString() + " of " + aliases; + public boolean supportsSkipLocked() { + return true; } @Override public String appendLockHint(LockOptions mode, String tableName) { - //TODO: is this really necessary??! - return tableName; - } - - @Override - public String applyLocksToSql(String sql, LockOptions aliasedLockOptions, Map keyColumnNames) { - //TODO: is this really correct? - return sql + new ForUpdateFragment( this, aliasedLockOptions, keyColumnNames ).toFragmentString(); + final String lockHint = super.appendLockHint( mode, tableName ); + return mode.getLockMode() == LockMode.UPGRADE_SKIPLOCKED || mode.getTimeOut() == LockOptions.SKIP_LOCKED + ? lockHint + " readpast" + : lockHint; } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/SybaseASESqlAstTranslator.java b/hibernate-core/src/main/java/org/hibernate/dialect/SybaseASESqlAstTranslator.java index 0dfc91292a..cc9c4a5910 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/SybaseASESqlAstTranslator.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/SybaseASESqlAstTranslator.java @@ -10,8 +10,10 @@ import java.util.List; import java.util.function.Consumer; import org.hibernate.LockMode; +import org.hibernate.LockOptions; import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.query.sqm.ComparisonOperator; +import org.hibernate.sql.ast.Clause; import org.hibernate.sql.ast.SqlAstJoinType; import org.hibernate.sql.ast.SqlAstNodeRenderingMode; import org.hibernate.sql.ast.spi.AbstractSqlAstTranslator; @@ -46,6 +48,8 @@ import org.hibernate.sql.exec.spi.JdbcOperation; */ public class SybaseASESqlAstTranslator extends AbstractSqlAstTranslator { + private static final String UNION_ALL = " union all "; + public SybaseASESqlAstTranslator(SessionFactoryImplementor sessionFactory, Statement statement) { super( sessionFactory, statement ); } @@ -109,8 +113,64 @@ public class SybaseASESqlAstTranslator extends Abstract @Override protected boolean renderNamedTableReference(NamedTableReference tableReference, LockMode lockMode) { - super.renderNamedTableReference( tableReference, lockMode ); - return false; + final String tableExpression = tableReference.getTableExpression(); + if ( tableReference instanceof UnionTableReference && lockMode != LockMode.NONE && tableExpression.charAt( 0 ) == '(' ) { + // SQL Server requires to push down the lock hint to the actual table names + int searchIndex = 0; + int unionIndex; + while ( ( unionIndex = tableExpression.indexOf( UNION_ALL, searchIndex ) ) != -1 ) { + append( tableExpression, searchIndex, unionIndex ); + renderLockHint( lockMode ); + appendSql( UNION_ALL ); + searchIndex = unionIndex + UNION_ALL.length(); + } + append( tableExpression, searchIndex, tableExpression.length() - 2 ); + renderLockHint( lockMode ); + appendSql( " )" ); + + registerAffectedTable( tableReference ); + final Clause currentClause = getClauseStack().getCurrent(); + if ( rendersTableReferenceAlias( currentClause ) ) { + final String identificationVariable = tableReference.getIdentificationVariable(); + if ( identificationVariable != null ) { + appendSql( ' ' ); + appendSql( identificationVariable ); + } + } + } + else { + super.renderNamedTableReference( tableReference, lockMode ); + renderLockHint( lockMode ); + } + // Just always return true because SQL Server doesn't support the FOR UPDATE clause + return true; + } + + private void renderLockHint(LockMode lockMode) { + final int effectiveLockTimeout = getEffectiveLockTimeout( lockMode ); + switch ( lockMode ) { + case PESSIMISTIC_READ: + case PESSIMISTIC_WRITE: + case WRITE: { + switch ( effectiveLockTimeout ) { + case LockOptions.SKIP_LOCKED: + appendSql( " holdlock readpast" ); + break; + default: + appendSql( " holdlock" ); + break; + } + break; + } + case UPGRADE_SKIPLOCKED: { + appendSql( " holdlock readpast" ); + break; + } + case UPGRADE_NOWAIT: { + appendSql( " holdlock" ); + break; + } + } } @Override @@ -144,6 +204,11 @@ public class SybaseASESqlAstTranslator extends Abstract } } + @Override + protected void renderForUpdateClause(QuerySpec querySpec, ForUpdateClause forUpdateClause) { + // Sybase ASE does not really support the FOR UPDATE clause + } + @Override protected void visitSqlSelections(SelectClause selectClause) { renderTopClause( (QuerySpec) getQueryPartStack().getCurrent(), true, false ); diff --git a/hibernate-core/src/main/java/org/hibernate/query/MutationQuery.java b/hibernate-core/src/main/java/org/hibernate/query/MutationQuery.java index ee8c7ff619..2f13e5178d 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/MutationQuery.java +++ b/hibernate-core/src/main/java/org/hibernate/query/MutationQuery.java @@ -14,7 +14,9 @@ import java.util.Map; import org.hibernate.FlushMode; import org.hibernate.Incubating; +import org.hibernate.Session; +import jakarta.persistence.FlushModeType; import jakarta.persistence.Parameter; import jakarta.persistence.TemporalType; @@ -79,6 +81,21 @@ public interface MutationQuery extends CommonQueryContract { // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ // Covariant returns + @Override + MutationQuery setFlushMode(FlushModeType flushMode); + + @Override + MutationQuery setHibernateFlushMode(FlushMode flushMode); + + @Override + MutationQuery setTimeout(int timeout); + + @Override + MutationQuery setComment(String comment); + + @Override + MutationQuery setHint(String hintName, Object value); + @Override MutationQuery setParameter(String name, Object value); @@ -192,7 +209,4 @@ public interface MutationQuery extends CommonQueryContract { @Override MutationQuery setProperties(@SuppressWarnings("rawtypes") Map bean); - - @Override - MutationQuery setHibernateFlushMode(FlushMode flushMode); } diff --git a/hibernate-core/src/main/java/org/hibernate/query/SelectionQuery.java b/hibernate-core/src/main/java/org/hibernate/query/SelectionQuery.java index 3d975d2fe3..caa66e1b54 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/SelectionQuery.java +++ b/hibernate-core/src/main/java/org/hibernate/query/SelectionQuery.java @@ -195,6 +195,9 @@ public interface SelectionQuery extends CommonQueryContract { @Override SelectionQuery setTimeout(int timeout); + @Override + SelectionQuery setComment(String comment); + /** * Obtain the JDBC fetch size hint in effect for this query. This value is eventually passed along to the JDBC * query via {@link java.sql.Statement#setFetchSize(int)}. As defined by JDBC, this value is a hint to the diff --git a/hibernate-core/src/main/java/org/hibernate/query/spi/AbstractSelectionQuery.java b/hibernate-core/src/main/java/org/hibernate/query/spi/AbstractSelectionQuery.java index 9c647ab352..c1166f5e38 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/spi/AbstractSelectionQuery.java +++ b/hibernate-core/src/main/java/org/hibernate/query/spi/AbstractSelectionQuery.java @@ -763,6 +763,12 @@ public abstract class AbstractSelectionQuery return this; } + @Override + public SelectionQuery setComment(String comment) { + super.setComment( comment ); + return this; + } + @Override public SelectionQuery setParameter(String name, Object value) { 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 56cbc2e873..612fce4a03 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 @@ -306,7 +306,7 @@ public class MatchingIdSelectionHelper { if ( !jdbcEnvironment.getDialect().supportsOuterJoinForUpdate() ) { matchingIdSelection.getQuerySpec().getFromClause().visitTableJoins( tableJoin -> { - if ( tableJoin.getJoinType() != SqlAstJoinType.INNER ) { + if ( tableJoin.isInitialized() && tableJoin.getJoinType() != SqlAstJoinType.INNER ) { lockOptions.setLockMode( lockMode ); } } diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/temptable/ExecuteWithTemporaryTableHelper.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/temptable/ExecuteWithTemporaryTableHelper.java index 3c66bd9984..dffb384204 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/temptable/ExecuteWithTemporaryTableHelper.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/temptable/ExecuteWithTemporaryTableHelper.java @@ -146,7 +146,7 @@ public final class ExecuteWithTemporaryTableHelper { querySpec -> { querySpec.getFromClause().visitTableJoins( tableJoin -> { - if ( tableJoin.getJoinType() != SqlAstJoinType.INNER ) { + if ( tableJoin.isInitialized() && tableJoin.getJoinType() != SqlAstJoinType.INNER ) { lockOptions.setLockMode( lockMode ); } } diff --git a/hibernate-core/src/main/java/org/hibernate/result/internal/OutputsImpl.java b/hibernate-core/src/main/java/org/hibernate/result/internal/OutputsImpl.java index f37802a123..a1e1c72d27 100644 --- a/hibernate-core/src/main/java/org/hibernate/result/internal/OutputsImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/result/internal/OutputsImpl.java @@ -219,8 +219,7 @@ public class OutputsImpl implements Outputs { final JdbcValuesSourceProcessingStateStandardImpl jdbcValuesSourceProcessingState = new JdbcValuesSourceProcessingStateStandardImpl( executionContext, - processingOptions, - executionContext::registerLoadingEntityEntry + processingOptions ); try { final RowProcessingStateStandardImpl rowProcessingState = new RowProcessingStateStandardImpl( 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 c4d71908de..89d068179e 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 @@ -1416,7 +1416,7 @@ public abstract class AbstractSqlAstTranslator implemen tableGroupJoin -> { final TableGroup group = tableGroupJoin.getJoinedGroup(); if ( forUpdateClause.hasAlias( group.getSourceAlias() ) ) { - if ( tableGroupJoin.getJoinType() != SqlAstJoinType.INNER && !( group instanceof VirtualTableGroup ) ) { + if ( tableGroupJoin.isInitialized() && tableGroupJoin.getJoinType() != SqlAstJoinType.INNER && !( group instanceof VirtualTableGroup ) ) { if ( Boolean.FALSE.equals( followOnLocking ) ) { throw new IllegalQueryOperationException( "Locking with OUTER joins is not supported" ); @@ -1434,7 +1434,7 @@ public abstract class AbstractSqlAstTranslator implemen // Visit TableReferenceJoin and TableGroupJoin to see if all use INNER if ( querySpec.getFromClause().queryTableJoins( tableJoin -> { - if ( tableJoin.getJoinType() != SqlAstJoinType.INNER && !( tableJoin.getJoinedNode() instanceof VirtualTableGroup ) ) { + if ( tableJoin.isInitialized() && tableJoin.getJoinType() != SqlAstJoinType.INNER && !( tableJoin.getJoinedNode() instanceof VirtualTableGroup ) ) { if ( Boolean.FALSE.equals( followOnLocking ) ) { throw new IllegalQueryOperationException( "Locking with OUTER joins is not supported" ); diff --git a/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/from/TableGroupJoin.java b/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/from/TableGroupJoin.java index eb34216500..0b3e9c4b37 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/from/TableGroupJoin.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/from/TableGroupJoin.java @@ -87,6 +87,11 @@ public class TableGroupJoin implements TableJoin, DomainResultProducer { sqlTreeWalker.visitTableGroupJoin( this ); } + @Override + public boolean isInitialized() { + return joinedGroup.isInitialized(); + } + public NavigablePath getNavigablePath() { return navigablePath; } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/from/TableJoin.java b/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/from/TableJoin.java index a28b9793fa..3b066bd62f 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/from/TableJoin.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/from/TableJoin.java @@ -19,4 +19,5 @@ public interface TableJoin extends SqlAstNode { SqlAstJoinType getJoinType(); Predicate getPredicate(); SqlAstNode getJoinedNode(); + boolean isInitialized(); } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/from/TableReferenceJoin.java b/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/from/TableReferenceJoin.java index 62a623f7a9..eb53c19750 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/from/TableReferenceJoin.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/from/TableReferenceJoin.java @@ -58,6 +58,11 @@ public class TableReferenceJoin implements TableJoin, PredicateContainer { return getJoinType().getText() + " join " + getJoinedTableReference().toString(); } + @Override + public boolean isInitialized() { + return true; + } + @Override public void applyPredicate(Predicate newPredicate) { predicate = SqlAstTreeHelper.combinePredicates( predicate, newPredicate); diff --git a/hibernate-core/src/main/java/org/hibernate/sql/exec/internal/JdbcSelectExecutorStandardImpl.java b/hibernate-core/src/main/java/org/hibernate/sql/exec/internal/JdbcSelectExecutorStandardImpl.java index 150ae44f61..2aadbddc5c 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/exec/internal/JdbcSelectExecutorStandardImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/exec/internal/JdbcSelectExecutorStandardImpl.java @@ -334,8 +334,7 @@ public class JdbcSelectExecutorStandardImpl implements JdbcSelectExecutor { final JdbcValuesSourceProcessingStateStandardImpl valuesProcessingState = new JdbcValuesSourceProcessingStateStandardImpl( executionContext, - processingOptions, - executionContext::registerLoadingEntityEntry + processingOptions ); final RowReader rowReader = ResultsHelper.createRowReader( diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/AbstractEntityInitializer.java b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/AbstractEntityInitializer.java index f649f1f887..8d9a72f6bb 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/AbstractEntityInitializer.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/AbstractEntityInitializer.java @@ -515,6 +515,7 @@ public abstract class AbstractEntityInitializer extends AbstractFetchParentAcces entityInstance = existingEntity; if ( existingLoadingEntry == null && isExistingEntityInitialized( existingEntity ) ) { this.isInitialized = true; + registerReloadedEntity( rowProcessingState, existingEntity ); notifyResolutionListeners( entityInstance ); } } @@ -661,6 +662,7 @@ public abstract class AbstractEntityInitializer extends AbstractFetchParentAcces // EARLY EXIT!!! // because the second level cache has reference cache entries, the entity is initialized isInitialized = true; + registerReloadedEntity( rowProcessingState, cached ); return cached; } } @@ -721,6 +723,17 @@ public abstract class AbstractEntityInitializer extends AbstractFetchParentAcces ); } + protected void registerReloadedEntity(RowProcessingState rowProcessingState, Object instance) { + if ( rowProcessingState.getCallback() != null ) { + // This is only needed for follow-on locking, so skip registering the entity if there is no callback + rowProcessingState.getJdbcValuesSourceProcessingState() + .registerReloadedEntity( + entityKey, + new LoadingEntityEntry( this, entityKey, concreteDescriptor, instance ) + ); + } + } + @Override public void initializeInstance(RowProcessingState rowProcessingState) { if ( !missing && !isInitialized ) { diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/jdbc/internal/JdbcValuesSourceProcessingStateStandardImpl.java b/hibernate-core/src/main/java/org/hibernate/sql/results/jdbc/internal/JdbcValuesSourceProcessingStateStandardImpl.java index 6db95a8287..3c602a5edb 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/jdbc/internal/JdbcValuesSourceProcessingStateStandardImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/jdbc/internal/JdbcValuesSourceProcessingStateStandardImpl.java @@ -10,7 +10,6 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.function.BiConsumer; import org.hibernate.engine.spi.CollectionKey; import org.hibernate.engine.spi.EntityKey; @@ -40,9 +39,8 @@ public class JdbcValuesSourceProcessingStateStandardImpl implements JdbcValuesSo private final ExecutionContext executionContext; private final JdbcValuesSourceProcessingOptions processingOptions; - private final BiConsumer loadingEntityEntryConsumer; - private Map loadingEntityMap; + private Map reloadedEntityMap; private Map initializerByUniquKeyMap; private Map loadingCollectionMap; private List arrayInitializers; @@ -52,11 +50,9 @@ public class JdbcValuesSourceProcessingStateStandardImpl implements JdbcValuesSo public JdbcValuesSourceProcessingStateStandardImpl( ExecutionContext executionContext, - JdbcValuesSourceProcessingOptions processingOptions, - BiConsumer loadingEntityEntryListener) { + JdbcValuesSourceProcessingOptions processingOptions) { this.executionContext = executionContext; this.processingOptions = processingOptions; - this.loadingEntityEntryConsumer = loadingEntityEntryListener; if ( executionContext.getSession().isEventSource() ) { final EventSource eventSource = executionContext.getSession().asEventSource(); @@ -101,12 +97,17 @@ public class JdbcValuesSourceProcessingStateStandardImpl implements JdbcValuesSo if ( loadingEntityMap == null ) { loadingEntityMap = new HashMap<>(); } + executionContext.registerLoadingEntityEntry( entityKey, loadingEntry ); + loadingEntityMap.put( entityKey, loadingEntry ); + } - if ( loadingEntityEntryConsumer != null ) { - loadingEntityEntryConsumer.accept( entityKey, loadingEntry ); + @Override + public void registerReloadedEntity(EntityKey entityKey, LoadingEntityEntry loadingEntry) { + if ( reloadedEntityMap == null ) { + reloadedEntityMap = new HashMap<>(); } - loadingEntityMap.put( entityKey, loadingEntry ); + reloadedEntityMap.put( entityKey, loadingEntry ); } @Override @@ -165,36 +166,53 @@ public class JdbcValuesSourceProcessingStateStandardImpl implements JdbcValuesSo } private void postLoad() { - if ( loadingEntityMap == null ) { - return; - } - final EventListenerGroup listenerGroup = executionContext.getSession().getFactory() - .getFastSessionServices() - .eventListenerGroup_POST_LOAD; + final Callback callback = executionContext.getCallback(); + if ( loadingEntityMap != null ) { + final EventListenerGroup listenerGroup = executionContext.getSession().getFactory() + .getFastSessionServices() + .eventListenerGroup_POST_LOAD; - loadingEntityMap.forEach( - (entityKey, loadingEntityEntry) -> { - if ( loadingEntityEntry.getEntityInstance() != null ) { - if ( postLoadEvent != null ) { - postLoadEvent.reset(); - postLoadEvent.setEntity( loadingEntityEntry.getEntityInstance() ) - .setId( entityKey.getIdentifier() ) - .setPersister( loadingEntityEntry.getDescriptor() ); - listenerGroup.fireEventOnEachListener( postLoadEvent, PostLoadEventListener::onPostLoad ); + loadingEntityMap.forEach( + (entityKey, loadingEntityEntry) -> { + if ( loadingEntityEntry.getEntityInstance() != null ) { + if ( postLoadEvent != null ) { + postLoadEvent.reset(); + postLoadEvent.setEntity( loadingEntityEntry.getEntityInstance() ) + .setId( entityKey.getIdentifier() ) + .setPersister( loadingEntityEntry.getDescriptor() ); + listenerGroup.fireEventOnEachListener( + postLoadEvent, + PostLoadEventListener::onPostLoad + ); + } + + if ( callback != null ) { + callback.invokeAfterLoadActions( + loadingEntityEntry.getEntityInstance(), + loadingEntityEntry.getDescriptor(), + getSession() + ); + } } + } + ); + } + loadingEntityMap = null; - final Callback callback = executionContext.getCallback(); - if ( callback != null ) { + if ( reloadedEntityMap != null ) { + if ( callback != null ) { + reloadedEntityMap.forEach( + (entityKey, loadingEntityEntry) -> { callback.invokeAfterLoadActions( loadingEntityEntry.getEntityInstance(), loadingEntityEntry.getDescriptor(), getSession() ); } - } - } - ); - loadingEntityMap = null; + ); + } + reloadedEntityMap = null; + } } @SuppressWarnings("SimplifiableIfStatement") diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/jdbc/spi/JdbcValuesSourceProcessingState.java b/hibernate-core/src/main/java/org/hibernate/sql/results/jdbc/spi/JdbcValuesSourceProcessingState.java index 1892aa5693..8de91f7e84 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/jdbc/spi/JdbcValuesSourceProcessingState.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/jdbc/spi/JdbcValuesSourceProcessingState.java @@ -55,6 +55,10 @@ public interface JdbcValuesSourceProcessingState { EntityKey entityKey, LoadingEntityEntry loadingEntry); + void registerReloadedEntity( + EntityKey entityKey, + LoadingEntityEntry loadingEntry); + void registerInitializer( EntityUniqueKey entityKey, Initializer initializer); diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/locking/jpa/FollowOnLockingTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/locking/jpa/FollowOnLockingTest.java index 30566a2a43..a57ea23e21 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/locking/jpa/FollowOnLockingTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/locking/jpa/FollowOnLockingTest.java @@ -7,6 +7,7 @@ package org.hibernate.orm.test.locking.jpa; import java.util.List; +import java.util.Map; import org.hibernate.query.spi.QueryImplementor; @@ -20,9 +21,11 @@ import org.junit.jupiter.api.Test; import jakarta.persistence.LockModeType; import jakarta.persistence.LockTimeoutException; import jakarta.persistence.PessimisticLockException; +import jakarta.persistence.QueryTimeoutException; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.fail; +import static org.hibernate.jpa.SpecHints.HINT_SPEC_QUERY_TIMEOUT; /** * @author Steve Ebersole @@ -88,12 +91,13 @@ public class FollowOnLockingTest { try { // with the initial txn still active (locks still held), try to update the row from another txn scope.inTransaction( (session2) -> { - final Employee june = session2.find( Employee.class, 3 ); - june.setSalary( 90000F ); + session2.createMutationQuery( "update Employee e set salary = 90000 where e.id = 3" ) + .setTimeout( 1 ) + .executeUpdate(); } ); fail( "Locked entity update was allowed" ); } - catch (PessimisticLockException | LockTimeoutException expected) { + catch (PessimisticLockException | LockTimeoutException | QueryTimeoutException expected) { } } ); diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/query/sqm/mutation/multitable/IdSelectionTests.java b/hibernate-core/src/test/java/org/hibernate/orm/test/query/sqm/mutation/multitable/IdSelectionTests.java index f1496c1c61..301a8cdc0e 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/query/sqm/mutation/multitable/IdSelectionTests.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/query/sqm/mutation/multitable/IdSelectionTests.java @@ -200,7 +200,7 @@ public class IdSelectionTests { @Override public Callback getCallback() { - throw new UnsupportedOperationException(); + return null; } } }