HHH-19506 Improve FirebirdDialect

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

@@ -16,6 +16,7 @@
 import org.hibernate.sql.ast.spi.SqlAppender;
 import org.hibernate.sql.ast.spi.SqlSelection;
 import org.hibernate.sql.ast.tree.Statement;
+import org.hibernate.sql.ast.tree.expression.BinaryArithmeticExpression;
 import org.hibernate.sql.ast.tree.expression.CaseSearchedExpression;
 import org.hibernate.sql.ast.tree.expression.CaseSimpleExpression;
 import org.hibernate.sql.ast.tree.expression.Expression;
@@ -27,13 +28,18 @@
 import org.hibernate.sql.ast.tree.expression.SelfRenderingExpression;
 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.ValuesTableReference;
+import org.hibernate.sql.ast.tree.insert.InsertSelectStatement;
 import org.hibernate.sql.ast.tree.predicate.BooleanExpressionPredicate;
+import org.hibernate.sql.ast.tree.predicate.ComparisonPredicate;
 import org.hibernate.sql.ast.tree.predicate.InListPredicate;
 import org.hibernate.sql.ast.tree.predicate.SelfRenderingPredicate;
 import org.hibernate.sql.ast.tree.select.QueryGroup;
 import org.hibernate.sql.ast.tree.select.QueryPart;
 import org.hibernate.sql.ast.tree.select.QuerySpec;
 import org.hibernate.sql.ast.tree.select.SelectClause;
+import org.hibernate.sql.ast.tree.update.UpdateStatement;
 import org.hibernate.sql.exec.spi.JdbcOperation;
 
 /**
@@ -49,6 +55,27 @@ public FirebirdSqlAstTranslator(SessionFactoryImplementor sessionFactory, Statem
 		super( sessionFactory, statement );
 	}
 
+	@Override
+	protected void visitInsertStatementOnly(InsertSelectStatement statement) {
+		if ( statement.getConflictClause() == null || statement.getConflictClause().isDoNothing() ) {
+			// Render plain insert statement and possibly run into unique constraint violation
+			super.visitInsertStatementOnly( statement );
+		}
+		else {
+			visitInsertStatementEmulateMerge( statement );
+		}
+	}
+
+	@Override
+	protected void visitUpdateStatementOnly(UpdateStatement statement) {
+		if ( hasNonTrivialFromClause( statement.getFromClause() ) ) {
+			visitUpdateStatementEmulateMerge( statement );
+		}
+		else {
+			super.visitUpdateStatementOnly( statement );
+		}
+	}
+
 	@Override
 	public void visitBooleanExpressionPredicate(BooleanExpressionPredicate booleanExpressionPredicate) {
 		if ( getDialect().getVersion().isSameOrAfter( 3 ) ) {
@@ -123,6 +150,11 @@ protected String getForUpdate() {
 		return " with lock";
 	}
 
+	@Override
+	protected String getForShare(int timeoutMillis) {
+		return getForUpdate();
+	}
+
 	protected boolean shouldEmulateFetchClause(QueryPart queryPart) {
 		// Percent fetches or ties fetches aren't supported in Firebird
 		// Before 3.0 there was also no support for window functions
@@ -151,6 +183,7 @@ public void visitQuerySpec(QuerySpec querySpec) {
 		}
 	}
 
+	// overridden due to visitSqlSelections
 	@Override
 	public void visitSelectClause(SelectClause selectClause) {
 		Stack<Clause> clauseStack = getClauseStack();
@@ -238,6 +271,11 @@ public void visitInListPredicate(InListPredicate inListPredicate) {
 		}
 	}
 
+	@Override
+	public void visitValuesTableReference(ValuesTableReference tableReference) {
+		emulateValuesTableReferenceColumnAliasing( tableReference );
+	}
+
 	private boolean supportsOffsetFetchClause() {
 		return getDialect().getVersion().isSameOrAfter( 3 );
 	}
@@ -259,7 +297,7 @@ public void visitSelfRenderingPredicate(SelfRenderingPredicate selfRenderingPred
 	public void visitSelfRenderingExpression(SelfRenderingExpression expression) {
 		// see comments in visitParameter
 		boolean inFunction = this.inFunction;
-		this.inFunction = !( expression instanceof FunctionExpression ) || !"cast".equals( ( (FunctionExpression) expression ).getFunctionName() );
+		this.inFunction = isFunctionOrCastWithArithmeticExpression( expression );
 		try {
 			super.visitSelfRenderingExpression( expression );
 		}
@@ -268,6 +306,14 @@ public void visitSelfRenderingExpression(SelfRenderingExpression expression) {
 		}
 	}
 
+	private boolean isFunctionOrCastWithArithmeticExpression(SelfRenderingExpression expression) {
+		// see comments in visitParameter
+		return expression instanceof FunctionExpression fe
+			&& ( !"cast".equals( fe.getFunctionName() )
+				// types of parameters in an arithmetic expression inside a cast are not (always?) inferred
+				|| fe.getArguments().get( 0 ) instanceof BinaryArithmeticExpression );
+	}
+
 	@Override
 	public void visitParameter(JdbcParameter jdbcParameter) {
 		if ( inFunction ) {
@@ -308,4 +354,38 @@ private void visitLiteral(Literal literal) {
 			renderLiteral( literal, inFunction );
 		}
 	}
+
+	@Override
+	public void visitRelationalPredicate(ComparisonPredicate comparisonPredicate) {
+		if ( isParameterOrContainsParameter( comparisonPredicate.getLeftHandExpression() )
+			&& isParameterOrContainsParameter( comparisonPredicate.getRightHandExpression() ) ) {
+			// Firebird cannot compare two parameters with each other as they will be untyped, and in some cases
+			// comparisons involving arithmetic expression with parameters also fails to infer the type
+			withParameterRenderingMode(
+					SqlAstNodeRenderingMode.NO_PLAIN_PARAMETER,
+					() -> super.visitRelationalPredicate( comparisonPredicate )
+			);
+		}
+		else {
+			super.visitRelationalPredicate( comparisonPredicate );
+		}
+	}
+
+	private static boolean isParameterOrContainsParameter( Expression expression ) {
+		if ( expression instanceof BinaryArithmeticExpression arithmeticExpression ) {
+			// Recursive search for parameter
+			return isParameterOrContainsParameter( arithmeticExpression.getLeftHandOperand() )
+				|| isParameterOrContainsParameter( arithmeticExpression.getRightHandOperand() );
+		}
+		return isParameter( expression );
+	}
+
+	@Override
+	protected void renderDmlTargetTableExpression(NamedTableReference tableReference) {
+		super.renderDmlTargetTableExpression( tableReference );
+		if ( getClauseStack().getCurrent() != Clause.INSERT ) {
+			renderTableReferenceIdentificationVariable( tableReference );
+		}
+	}
+
 }

hibernate-core/src/main/java/org/hibernate/dialect/function/CommonFunctionFactory.java

@@ -747,7 +747,11 @@ public void repeat() {
 	}
 
 	public void repeat_rpad() {
-		functionRegistry.patternDescriptorBuilder( "repeat", "rpad(?1,?2*length(?1),?1)" )
+		repeat_rpad( "length" );
+	}
+
+	public void repeat_rpad(String lengthFunctionName) {
+		functionRegistry.patternDescriptorBuilder( "repeat", "rpad(?1,?2*" + lengthFunctionName + "(?1),?1)" )
 				.setInvariantType(stringType)
 				.setExactArgumentCount( 2 )
 				.setParameterTypes(STRING, INTEGER)

hibernate-core/src/main/java/org/hibernate/engine/jdbc/env/internal/JdbcEnvironmentInitiator.java

@@ -402,10 +402,11 @@ private int databaseMicroVersion(DatabaseMetaData metadata) throws SQLException
 							final String version = metadata.getDatabaseProductVersion();
 							final String prefix =
 									metadata.getDatabaseMajorVersion() + "." + metadata.getDatabaseMinorVersion() + ".";
-							if ( version.startsWith(prefix) ) {
+							final int versionIndex = version.indexOf( prefix );
+							if ( versionIndex >= 0 ) {
 								try {
-									final String substring = version.substring( prefix.length() );
-									final String micro = new StringTokenizer(substring," .,-:;/()[]").nextToken();
+									final String substring = version.substring( versionIndex + prefix.length() );
+									final String micro = new StringTokenizer( substring, " .,-:;/()[]" ).nextToken();
 									return parseInt(micro);
 								}
 								catch (NumberFormatException nfe) {

hibernate-core/src/test/java/org/hibernate/orm/test/insertordering/BaseInsertOrderingTest.java

@@ -6,6 +6,7 @@
 
 import java.sql.PreparedStatement;
 import java.sql.Types;
+import java.util.Collection;
 import java.util.List;
 
 import org.hibernate.boot.registry.StandardServiceRegistryBuilder;
@@ -28,15 +29,26 @@
 abstract class BaseInsertOrderingTest extends BaseSessionFactoryFunctionalTest {
 
 	static class Batch {
-		String sql;
+		Collection<String> sql;
 		int size;
 
 		Batch(String sql, int size) {
+			this(List.of(sql), size);
+		}
+
+		Batch(String sql) {
+			this( sql, 1 );
+		}
+
+		Batch(Collection<String> sql, int size) {
+			if (sql.isEmpty()) {
+				throw new IllegalArgumentException( "At least one expected statement is required" );
+			}
 			this.sql = sql;
 			this.size = size;
 		}
 
-		Batch(String sql) {
+		Batch(Collection<String> sql) {
 			this( sql, 1 );
 		}
 	}
@@ -76,7 +88,7 @@ protected String literal(String value) {
 
 	void verifyContainsBatches(Batch... expectedBatches) {
 		for ( Batch expectedBatch : expectedBatches ) {
-			PreparedStatement preparedStatement = connectionProvider.getPreparedStatement( expectedBatch.sql );
+			PreparedStatement preparedStatement = findPreparedStatement( expectedBatch );
 			try {
 				List<Object[]> addBatchCalls = connectionProvider.spyContext.getCalls(
 						PreparedStatement.class.getMethod( "addBatch" ),
@@ -95,6 +107,25 @@ void verifyContainsBatches(Batch... expectedBatches) {
 		}
 	}
 
+	private PreparedStatement findPreparedStatement(Batch expectedBatch) {
+		IllegalArgumentException firstException = null;
+		for (String sql : expectedBatch.sql) {
+			try {
+				return connectionProvider.getPreparedStatement( sql );
+			}
+			catch ( IllegalArgumentException e ) {
+				if ( firstException == null ) {
+					firstException = e;
+				} else {
+					firstException.addSuppressed( e );
+				}
+			}
+		}
+		throw firstException != null
+				? firstException
+				: new IllegalArgumentException( "No prepared statement found as none were expected" );
+	}
+
 	void verifyPreparedStatementCount(int expectedBatchCount) {
 		final int realBatchCount = connectionProvider.getPreparedSQLStatements().size();
 		assertThat( realBatchCount )

hibernate-core/src/test/java/org/hibernate/orm/test/insertordering/InsertOrderingSelfReferenceTest.java

@@ -78,8 +78,13 @@ public void testReferenceItself() {
 
 		verifyContainsBatches(
 				new Batch( "insert into Placeholder (name,id) values (?,?)", 2 ),
-				new Batch( "insert into Parameter (name,parent_id,TYPE,id) values (?,?," + literal( "INPUT" ) + ",?)" ),
-				new Batch( "insert into Parameter (name,parent_id,TYPE,id) values (?,?," + literal( "OUTPUT" ) + ",?)", 3 )
+				new Batch( List.of(
+						"insert into Parameter (name,parent_id,TYPE,id) values (?,?," + literal( "INPUT" ) + ",?)",
+						"insert into \"Parameter\" (name,\"parent_id\",TYPE,id) values (?,?," + literal( "INPUT" ) + ",?)" ) ),
+				new Batch( List.of(
+						"insert into Parameter (name,parent_id,TYPE,id) values (?,?," + literal( "OUTPUT" ) + ",?)",
+						"insert into \"Parameter\" (name,\"parent_id\",TYPE,id) values (?,?," + literal( "OUTPUT" ) + ",?)" ),
+						3 )
 		);
 	}
 

hibernate-core/src/test/java/org/hibernate/orm/test/jpa/BaseEntityManagerFunctionalTestCase.java

@@ -298,19 +298,27 @@ private void releaseUnclosedEntityManager(EntityManager em) {
 		if ( em == null ) {
 			return;
 		}
-		if ( !em.isOpen() ) {
-			return;
-		}
-
-		if ( em.getTransaction().isActive() ) {
-			em.getTransaction().rollback();
-			log.warn("You left an open transaction! Fix your test case. For now, we are closing it for you.");
+		try {
+			if ( !em.isOpen() ) {
+				return;
+			}
+			try {
+				if ( em.getTransaction().isActive() ) {
+					log.warn( "You left an open transaction! Fix your test case. For now, we are closing it for you." );
+					em.getTransaction().rollback();
+				}
+			}
+			finally {
+				if ( em.isOpen() ) {
+					// as we open an EM before the test runs, it will still be open if the test uses a custom EM.
+					// or, the person may have forgotten to close. So, do not raise a "fail", but log the fact.
+					log.warn( "The EntityManager is not closed. Closing it." );
+					em.close();
+				}
+			}
 		}
-		if ( em.isOpen() ) {
-			// as we open an EM before the test runs, it will still be open if the test uses a custom EM.
-			// or, the person may have forgotten to close. So, do not raise a "fail", but log the fact.
-			em.close();
-			log.warn("The EntityManager is not closed. Closing it.");
+		catch (RuntimeException e) {
+			log.debug( "Ignoring exception during clean up", e );
 		}
 	}
 

hibernate-core/src/test/java/org/hibernate/orm/test/jpa/criteria/subquery/SubqueryTests.java

@@ -94,7 +94,8 @@ public void testNoFromClauseInSubquery(EntityManagerFactoryScope scope) {
 					assertEquals("Jack", entities.get(0).getName());
 					assertEquals("Black", entities.get(0).getSurName());
 					assertEquals("John", entities.get(1).getName());
-					assertEquals("Doe", entities.get(1).getSurName());
+					// Trim because in some dialects, the type is CHAR(5), leading to trailing spaces
+					assertEquals("Doe", entities.get(1).getSurName().trim());
 				}
 		);
 	}

hibernate-core/src/test/java/org/hibernate/orm/test/jpa/metadata/TypesafeNamedQueryTest.java

@@ -4,6 +4,7 @@
  */
 package org.hibernate.orm.test.jpa.metadata;
 
+import org.hibernate.community.dialect.FirebirdDialect;
 import org.hibernate.dialect.SybaseASEDialect;
 import org.hibernate.testing.orm.junit.EntityManagerFactoryScope;
 import org.hibernate.testing.orm.junit.Jpa;
@@ -16,8 +17,8 @@
 
 @Jpa(annotatedClasses = Record.class)
 public class TypesafeNamedQueryTest {
-	@SkipForDialect(dialectClass = SybaseASEDialect.class,
-			reason = "'order by timestamp, id' not quite working")
+	@SkipForDialect(dialectClass = SybaseASEDialect.class, reason = "'order by timestamp, id' not quite working")
+	@SkipForDialect(dialectClass = FirebirdDialect.class, reason = "'order by timestamp, id' not quite working")
 	@Test void test(EntityManagerFactoryScope scope) {
 		scope.inTransaction( entityManager -> {
 			Record record1 = new Record("Hello, World!");

hibernate-core/src/test/java/org/hibernate/orm/test/loading/multiLoad/MultiLoadLockingTest.java

@@ -35,6 +35,8 @@
 import java.util.List;
 import java.util.function.Function;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -369,9 +371,9 @@ void testMultiLoadSimpleIdEntityOptimisticForceIncrementLock(SessionFactoryScope
 	}
 
 	private void checkStatement(int stmtCount, String lockString) {
-		assertEquals( stmtCount,sqlStatementInspector.getSqlQueries().size() );
+		assertEquals( stmtCount, sqlStatementInspector.getSqlQueries().size() );
 		for ( String stmt : sqlStatementInspector.getSqlQueries() ) {
-			assertTrue( stmt.contains( lockString ) );
+			assertThat( stmt, containsString( lockString ) );
 		}
 		sqlStatementInspector.clear();
 	}

hibernate-core/src/test/java/org/hibernate/orm/test/query/criteria/CriteriaBuilderNonStandardFunctionsTest.java

@@ -16,7 +16,6 @@
 
 import jakarta.persistence.criteria.ParameterExpression;
 import org.hibernate.dialect.CockroachDialect;
-import org.hibernate.community.dialect.DerbyDialect;
 import org.hibernate.dialect.PostgreSQLDialect;
 import org.hibernate.query.criteria.HibernateCriteriaBuilder;
 import org.hibernate.query.criteria.JpaExpression;
@@ -479,7 +478,7 @@ public void testNumericTruncFunction(SessionFactoryScope scope) {
 
 	@Test
 	@JiraKey("HHH-16130")
-	@SkipForDialect(dialectClass = DerbyDialect.class, reason = "Derby doesn't support any form of date truncation")
+	@RequiresDialectFeature( feature = DialectFeatureChecks.SupportsDateTimeTruncation.class )
 	public void testDateTruncFunction(SessionFactoryScope scope) {
 		scope.inTransaction( session -> {
 			HibernateCriteriaBuilder cb = session.getCriteriaBuilder();