From 7823b48a3a18426947a63b90df868063485a101f Mon Sep 17 00:00:00 2001 From: Christian Beikov Date: Wed, 17 May 2023 11:40:07 +0200 Subject: [PATCH] HHH-16541 Fix Sybase test issues and HSQLDB hanging --- .../SybaseASELegacySqlAstTranslator.java | 2 +- .../SybaseAnywhereSqlAstTranslator.java | 42 +++++++++++++++++-- .../dialect/SybaseLegacySqlAstTranslator.java | 40 +++++++++++++++++- .../dialect/SybaseASESqlAstTranslator.java | 2 +- .../dialect/SybaseSqlAstTranslator.java | 41 +++++++++++++++++- .../test/locking/jpa/FollowOnLockingTest.java | 3 ++ 6 files changed, 121 insertions(+), 9 deletions(-) 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 947b09832f..17b1e501f7 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 @@ -124,7 +124,7 @@ public class SybaseASELegacySqlAstTranslator extends Ab appendSql( UNION_ALL ); searchIndex = unionIndex + UNION_ALL.length(); } - append( tableExpression, searchIndex, tableExpression.length() - 2 ); + append( tableExpression, searchIndex, tableExpression.length() - 1 ); renderLockHint( lockMode ); appendSql( " )" ); diff --git a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/SybaseAnywhereSqlAstTranslator.java b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/SybaseAnywhereSqlAstTranslator.java index 9bcc5a05c7..56de0b9622 100644 --- a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/SybaseAnywhereSqlAstTranslator.java +++ b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/SybaseAnywhereSqlAstTranslator.java @@ -12,6 +12,7 @@ import java.util.function.Consumer; import org.hibernate.LockMode; import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.query.sqm.ComparisonOperator; +import org.hibernate.sql.ast.Clause; import org.hibernate.sql.ast.SqlAstNodeRenderingMode; import org.hibernate.sql.ast.spi.AbstractSqlAstTranslator; import org.hibernate.sql.ast.spi.SqlSelection; @@ -25,6 +26,7 @@ import org.hibernate.sql.ast.tree.expression.Literal; import org.hibernate.sql.ast.tree.expression.SqlTuple; import org.hibernate.sql.ast.tree.expression.Summarization; import org.hibernate.sql.ast.tree.from.NamedTableReference; +import org.hibernate.sql.ast.tree.from.UnionTableReference; import org.hibernate.sql.ast.tree.select.QueryPart; import org.hibernate.sql.ast.tree.select.QuerySpec; import org.hibernate.sql.ast.tree.select.SelectClause; @@ -37,6 +39,8 @@ import org.hibernate.sql.exec.spi.JdbcOperation; */ public class SybaseAnywhereSqlAstTranslator extends AbstractSqlAstTranslator { + private static final String UNION_ALL = " union all "; + public SybaseAnywhereSqlAstTranslator(SessionFactoryImplementor sessionFactory, Statement statement) { super( sessionFactory, statement ); } @@ -95,16 +99,48 @@ public class SybaseAnywhereSqlAstTranslator extends Abs @Override protected boolean renderNamedTableReference(NamedTableReference tableReference, LockMode lockMode) { - super.renderNamedTableReference( tableReference, lockMode ); if ( getDialect().getVersion().isBefore( 10 ) ) { - 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() - 1 ); + 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; } return false; } + private void renderLockHint(LockMode lockMode) { + if ( LockMode.READ.lessThan( lockMode ) ) { + appendSql( " holdlock" ); + } + } + @Override protected void renderForUpdateClause(QuerySpec querySpec, ForUpdateClause forUpdateClause) { if ( getDialect().getVersion().isBefore( 10 ) ) { diff --git a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/SybaseLegacySqlAstTranslator.java b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/SybaseLegacySqlAstTranslator.java index c4424d5120..faaec68593 100644 --- a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/SybaseLegacySqlAstTranslator.java +++ b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/SybaseLegacySqlAstTranslator.java @@ -12,6 +12,7 @@ import java.util.function.Consumer; import org.hibernate.LockMode; import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.query.sqm.ComparisonOperator; +import org.hibernate.sql.ast.Clause; import org.hibernate.sql.ast.SqlAstNodeRenderingMode; import org.hibernate.sql.ast.spi.AbstractSqlAstTranslator; import org.hibernate.sql.ast.spi.SqlSelection; @@ -25,6 +26,7 @@ import org.hibernate.sql.ast.tree.expression.Literal; import org.hibernate.sql.ast.tree.expression.SqlTuple; import org.hibernate.sql.ast.tree.expression.Summarization; import org.hibernate.sql.ast.tree.from.NamedTableReference; +import org.hibernate.sql.ast.tree.from.UnionTableReference; import org.hibernate.sql.ast.tree.select.QueryPart; import org.hibernate.sql.ast.tree.select.QuerySpec; import org.hibernate.sql.exec.spi.JdbcOperation; @@ -36,6 +38,8 @@ import org.hibernate.sql.exec.spi.JdbcOperation; */ public class SybaseLegacySqlAstTranslator extends AbstractSqlAstTranslator { + private static final String UNION_ALL = " union all "; + public SybaseLegacySqlAstTranslator(SessionFactoryImplementor sessionFactory, Statement statement) { super( sessionFactory, statement ); } @@ -99,11 +103,43 @@ public class SybaseLegacySqlAstTranslator extends Abstr @Override protected boolean renderNamedTableReference(NamedTableReference tableReference, LockMode lockMode) { - super.renderNamedTableReference( tableReference, lockMode ); + 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() - 1 ); + 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) { if ( LockMode.READ.lessThan( lockMode ) ) { appendSql( " holdlock" ); } - return true; } @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 cc9c4a5910..5782f7399c 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/SybaseASESqlAstTranslator.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/SybaseASESqlAstTranslator.java @@ -124,7 +124,7 @@ public class SybaseASESqlAstTranslator extends Abstract appendSql( UNION_ALL ); searchIndex = unionIndex + UNION_ALL.length(); } - append( tableExpression, searchIndex, tableExpression.length() - 2 ); + append( tableExpression, searchIndex, tableExpression.length() - 1 ); renderLockHint( lockMode ); appendSql( " )" ); diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/SybaseSqlAstTranslator.java b/hibernate-core/src/main/java/org/hibernate/dialect/SybaseSqlAstTranslator.java index faa7770657..e74bf6c764 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/SybaseSqlAstTranslator.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/SybaseSqlAstTranslator.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.SqlAstNodeRenderingMode; import org.hibernate.sql.ast.spi.AbstractSqlAstTranslator; import org.hibernate.sql.ast.spi.SqlSelection; @@ -25,6 +27,7 @@ import org.hibernate.sql.ast.tree.expression.Literal; import org.hibernate.sql.ast.tree.expression.SqlTuple; import org.hibernate.sql.ast.tree.expression.Summarization; import org.hibernate.sql.ast.tree.from.NamedTableReference; +import org.hibernate.sql.ast.tree.from.UnionTableReference; import org.hibernate.sql.ast.tree.select.QueryPart; import org.hibernate.sql.ast.tree.select.QuerySpec; import org.hibernate.sql.exec.spi.JdbcOperation; @@ -36,6 +39,8 @@ import org.hibernate.sql.exec.spi.JdbcOperation; */ public class SybaseSqlAstTranslator extends AbstractSqlAstTranslator { + private static final String UNION_ALL = " union all "; + public SybaseSqlAstTranslator(SessionFactoryImplementor sessionFactory, Statement statement) { super( sessionFactory, statement ); } @@ -99,11 +104,43 @@ public class SybaseSqlAstTranslator extends AbstractSql @Override protected boolean renderNamedTableReference(NamedTableReference tableReference, LockMode lockMode) { - super.renderNamedTableReference( tableReference, lockMode ); + 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() - 1 ); + 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) { if ( LockMode.READ.lessThan( lockMode ) ) { appendSql( " holdlock" ); } - return true; } @Override 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 a57ea23e21..9d8184f828 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 @@ -9,12 +9,14 @@ package org.hibernate.orm.test.locking.jpa; import java.util.List; import java.util.Map; +import org.hibernate.dialect.HSQLDialect; import org.hibernate.query.spi.QueryImplementor; import org.hibernate.testing.jdbc.SQLStatementInspector; import org.hibernate.testing.orm.junit.DomainModel; import org.hibernate.testing.orm.junit.SessionFactory; import org.hibernate.testing.orm.junit.SessionFactoryScope; +import org.hibernate.testing.orm.junit.SkipForDialect; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; @@ -32,6 +34,7 @@ import static org.hibernate.jpa.SpecHints.HINT_SPEC_QUERY_TIMEOUT; */ @DomainModel(annotatedClasses = { Employee.class, Department.class }) @SessionFactory(useCollectingStatementInspector = true) +@SkipForDialect(dialectClass = HSQLDialect.class, reason = "Seems HSQLDB doesn't cancel the query if it waits for a lock?!") public class FollowOnLockingTest { @Test