Skip to content

Commit 3a1d5fa

Browse files
mrotteveelmrotteveel
committed
HHH-19506 Improve FirebirdDialect
Including changes to either ignore tests, or get tests running on Firebird
1 parent 651b0af commit 3a1d5fa

29 files changed

+523
-221
lines changed

‎hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/FirebirdDialect.java

Lines changed: 175 additions & 105 deletions
Large diffs are not rendered by default.

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

Lines changed: 81 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import org.hibernate.sql.ast.spi.SqlAppender;
1717
import org.hibernate.sql.ast.spi.SqlSelection;
1818
import org.hibernate.sql.ast.tree.Statement;
19+
import org.hibernate.sql.ast.tree.expression.BinaryArithmeticExpression;
1920
import org.hibernate.sql.ast.tree.expression.CaseSearchedExpression;
2021
import org.hibernate.sql.ast.tree.expression.CaseSimpleExpression;
2122
import org.hibernate.sql.ast.tree.expression.Expression;
@@ -27,13 +28,18 @@
2728
import org.hibernate.sql.ast.tree.expression.SelfRenderingExpression;
2829
import org.hibernate.sql.ast.tree.expression.SqlTuple;
2930
import org.hibernate.sql.ast.tree.expression.Summarization;
31+
import org.hibernate.sql.ast.tree.from.NamedTableReference;
32+
import org.hibernate.sql.ast.tree.from.ValuesTableReference;
33+
import org.hibernate.sql.ast.tree.insert.InsertSelectStatement;
3034
import org.hibernate.sql.ast.tree.predicate.BooleanExpressionPredicate;
35+
import org.hibernate.sql.ast.tree.predicate.ComparisonPredicate;
3136
import org.hibernate.sql.ast.tree.predicate.InListPredicate;
3237
import org.hibernate.sql.ast.tree.predicate.SelfRenderingPredicate;
3338
import org.hibernate.sql.ast.tree.select.QueryGroup;
3439
import org.hibernate.sql.ast.tree.select.QueryPart;
3540
import org.hibernate.sql.ast.tree.select.QuerySpec;
3641
import org.hibernate.sql.ast.tree.select.SelectClause;
42+
import org.hibernate.sql.ast.tree.update.UpdateStatement;
3743
import org.hibernate.sql.exec.spi.JdbcOperation;
3844

3945
/**
@@ -49,6 +55,27 @@ public FirebirdSqlAstTranslator(SessionFactoryImplementor sessionFactory, Statem
4955
super( sessionFactory, statement );
5056
}
5157

58+
@Override
59+
protected void visitInsertStatementOnly(InsertSelectStatement statement) {
60+
if ( statement.getConflictClause() == null || statement.getConflictClause().isDoNothing() ) {
61+
// Render plain insert statement and possibly run into unique constraint violation
62+
super.visitInsertStatementOnly( statement );
63+
}
64+
else {
65+
visitInsertStatementEmulateMerge( statement );
66+
}
67+
}
68+
69+
@Override
70+
protected void visitUpdateStatementOnly(UpdateStatement statement) {
71+
if ( hasNonTrivialFromClause( statement.getFromClause() ) ) {
72+
visitUpdateStatementEmulateMerge( statement );
73+
}
74+
else {
75+
super.visitUpdateStatementOnly( statement );
76+
}
77+
}
78+
5279
@Override
5380
public void visitBooleanExpressionPredicate(BooleanExpressionPredicate booleanExpressionPredicate) {
5481
if ( getDialect().getVersion().isSameOrAfter( 3 ) ) {
@@ -123,6 +150,11 @@ protected String getForUpdate() {
123150
return " with lock";
124151
}
125152

153+
@Override
154+
protected String getForShare(int timeoutMillis) {
155+
return getForUpdate();
156+
}
157+
126158
protected boolean shouldEmulateFetchClause(QueryPart queryPart) {
127159
// Percent fetches or ties fetches aren't supported in Firebird
128160
// Before 3.0 there was also no support for window functions
@@ -151,6 +183,7 @@ public void visitQuerySpec(QuerySpec querySpec) {
151183
}
152184
}
153185

186+
// overridden due to visitSqlSelections
154187
@Override
155188
public void visitSelectClause(SelectClause selectClause) {
156189
Stack<Clause> clauseStack = getClauseStack();
@@ -238,6 +271,11 @@ public void visitInListPredicate(InListPredicate inListPredicate) {
238271
}
239272
}
240273

274+
@Override
275+
public void visitValuesTableReference(ValuesTableReference tableReference) {
276+
emulateValuesTableReferenceColumnAliasing( tableReference );
277+
}
278+
241279
private boolean supportsOffsetFetchClause() {
242280
return getDialect().getVersion().isSameOrAfter( 3 );
243281
}
@@ -259,7 +297,7 @@ public void visitSelfRenderingPredicate(SelfRenderingPredicate selfRenderingPred
259297
public void visitSelfRenderingExpression(SelfRenderingExpression expression) {
260298
// see comments in visitParameter
261299
boolean inFunction = this.inFunction;
262-
this.inFunction = !( expression instanceof FunctionExpression ) || !"cast".equals( ( (FunctionExpression) expression ).getFunctionName() );
300+
this.inFunction = isFunctionOrCastWithArithmeticExpression( expression );
263301
try {
264302
super.visitSelfRenderingExpression( expression );
265303
}
@@ -268,6 +306,14 @@ public void visitSelfRenderingExpression(SelfRenderingExpression expression) {
268306
}
269307
}
270308

309+
private boolean isFunctionOrCastWithArithmeticExpression(SelfRenderingExpression expression) {
310+
// see comments in visitParameter
311+
return expression instanceof FunctionExpression fe
312+
&& ( !"cast".equals( fe.getFunctionName() )
313+
// types of parameters in an arithmetic expression inside a cast are not (always?) inferred
314+
|| fe.getArguments().get( 0 ) instanceof BinaryArithmeticExpression );
315+
}
316+
271317
@Override
272318
public void visitParameter(JdbcParameter jdbcParameter) {
273319
if ( inFunction ) {
@@ -308,4 +354,38 @@ private void visitLiteral(Literal literal) {
308354
renderLiteral( literal, inFunction );
309355
}
310356
}
357+
358+
@Override
359+
public void visitRelationalPredicate(ComparisonPredicate comparisonPredicate) {
360+
if ( isParameterOrContainsParameter( comparisonPredicate.getLeftHandExpression() )
361+
&& isParameterOrContainsParameter( comparisonPredicate.getRightHandExpression() ) ) {
362+
// Firebird cannot compare two parameters with each other as they will be untyped, and in some cases
363+
// comparisons involving arithmetic expression with parameters also fails to infer the type
364+
withParameterRenderingMode(
365+
SqlAstNodeRenderingMode.NO_PLAIN_PARAMETER,
366+
() -> super.visitRelationalPredicate( comparisonPredicate )
367+
);
368+
}
369+
else {
370+
super.visitRelationalPredicate( comparisonPredicate );
371+
}
372+
}
373+
374+
private static boolean isParameterOrContainsParameter( Expression expression ) {
375+
if ( expression instanceof BinaryArithmeticExpression arithmeticExpression ) {
376+
// Recursive search for parameter
377+
return isParameterOrContainsParameter( arithmeticExpression.getLeftHandOperand() )
378+
|| isParameterOrContainsParameter( arithmeticExpression.getRightHandOperand() );
379+
}
380+
return isParameter( expression );
381+
}
382+
383+
@Override
384+
protected void renderDmlTargetTableExpression(NamedTableReference tableReference) {
385+
super.renderDmlTargetTableExpression( tableReference );
386+
if ( getClauseStack().getCurrent() != Clause.INSERT ) {
387+
renderTableReferenceIdentificationVariable( tableReference );
388+
}
389+
}
390+
311391
}

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -747,7 +747,11 @@ public void repeat() {
747747
}
748748

749749
public void repeat_rpad() {
750-
functionRegistry.patternDescriptorBuilder( "repeat", "rpad(?1,?2*length(?1),?1)" )
750+
repeat_rpad( "length" );
751+
}
752+
753+
public void repeat_rpad(String lengthFunctionName) {
754+
functionRegistry.patternDescriptorBuilder( "repeat", "rpad(?1,?2*" + lengthFunctionName + "(?1),?1)" )
751755
.setInvariantType(stringType)
752756
.setExactArgumentCount( 2 )
753757
.setParameterTypes(STRING, INTEGER)

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -402,10 +402,11 @@ private int databaseMicroVersion(DatabaseMetaData metadata) throws SQLException
402402
final String version = metadata.getDatabaseProductVersion();
403403
final String prefix =
404404
metadata.getDatabaseMajorVersion() + "." + metadata.getDatabaseMinorVersion() + ".";
405-
if ( version.startsWith(prefix) ) {
405+
final int versionIndex = version.indexOf( prefix );
406+
if ( versionIndex >= 0 ) {
406407
try {
407-
final String substring = version.substring( prefix.length() );
408-
final String micro = new StringTokenizer(substring," .,-:;/()[]").nextToken();
408+
final String substring = version.substring( versionIndex + prefix.length() );
409+
final String micro = new StringTokenizer( substring, " .,-:;/()[]" ).nextToken();
409410
return parseInt(micro);
410411
}
411412
catch (NumberFormatException nfe) {

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

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
import java.sql.PreparedStatement;
88
import java.sql.Types;
9+
import java.util.Collection;
910
import java.util.List;
1011

1112
import org.hibernate.boot.registry.StandardServiceRegistryBuilder;
@@ -28,15 +29,26 @@
2829
abstract class BaseInsertOrderingTest extends BaseSessionFactoryFunctionalTest {
2930

3031
static class Batch {
31-
String sql;
32+
Collection<String> sql;
3233
int size;
3334

3435
Batch(String sql, int size) {
36+
this(List.of(sql), size);
37+
}
38+
39+
Batch(String sql) {
40+
this( sql, 1 );
41+
}
42+
43+
Batch(Collection<String> sql, int size) {
44+
if (sql.isEmpty()) {
45+
throw new IllegalArgumentException( "At least one expected statement is required" );
46+
}
3547
this.sql = sql;
3648
this.size = size;
3749
}
3850

39-
Batch(String sql) {
51+
Batch(Collection<String> sql) {
4052
this( sql, 1 );
4153
}
4254
}
@@ -76,7 +88,7 @@ protected String literal(String value) {
7688

7789
void verifyContainsBatches(Batch... expectedBatches) {
7890
for ( Batch expectedBatch : expectedBatches ) {
79-
PreparedStatement preparedStatement = connectionProvider.getPreparedStatement( expectedBatch.sql );
91+
PreparedStatement preparedStatement = findPreparedStatement( expectedBatch );
8092
try {
8193
List<Object[]> addBatchCalls = connectionProvider.spyContext.getCalls(
8294
PreparedStatement.class.getMethod( "addBatch" ),
@@ -95,6 +107,25 @@ void verifyContainsBatches(Batch... expectedBatches) {
95107
}
96108
}
97109

110+
private PreparedStatement findPreparedStatement(Batch expectedBatch) {
111+
IllegalArgumentException firstException = null;
112+
for (String sql : expectedBatch.sql) {
113+
try {
114+
return connectionProvider.getPreparedStatement( sql );
115+
}
116+
catch ( IllegalArgumentException e ) {
117+
if ( firstException == null ) {
118+
firstException = e;
119+
} else {
120+
firstException.addSuppressed( e );
121+
}
122+
}
123+
}
124+
throw firstException != null
125+
? firstException
126+
: new IllegalArgumentException( "No prepared statement found as none were expected" );
127+
}
128+
98129
void verifyPreparedStatementCount(int expectedBatchCount) {
99130
final int realBatchCount = connectionProvider.getPreparedSQLStatements().size();
100131
assertThat( realBatchCount )

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,13 @@ public void testReferenceItself() {
7878

7979
verifyContainsBatches(
8080
new Batch( "insert into Placeholder (name,id) values (?,?)", 2 ),
81-
new Batch( "insert into Parameter (name,parent_id,TYPE,id) values (?,?," + literal( "INPUT" ) + ",?)" ),
82-
new Batch( "insert into Parameter (name,parent_id,TYPE,id) values (?,?," + literal( "OUTPUT" ) + ",?)", 3 )
81+
new Batch( List.of(
82+
"insert into Parameter (name,parent_id,TYPE,id) values (?,?," + literal( "INPUT" ) + ",?)",
83+
"insert into \"Parameter\" (name,\"parent_id\",TYPE,id) values (?,?," + literal( "INPUT" ) + ",?)" ) ),
84+
new Batch( List.of(
85+
"insert into Parameter (name,parent_id,TYPE,id) values (?,?," + literal( "OUTPUT" ) + ",?)",
86+
"insert into \"Parameter\" (name,\"parent_id\",TYPE,id) values (?,?," + literal( "OUTPUT" ) + ",?)" ),
87+
3 )
8388
);
8489
}
8590

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

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -298,19 +298,27 @@ private void releaseUnclosedEntityManager(EntityManager em) {
298298
if ( em == null ) {
299299
return;
300300
}
301-
if ( !em.isOpen() ) {
302-
return;
303-
}
304-
305-
if ( em.getTransaction().isActive() ) {
306-
em.getTransaction().rollback();
307-
log.warn("You left an open transaction! Fix your test case. For now, we are closing it for you.");
301+
try {
302+
if ( !em.isOpen() ) {
303+
return;
304+
}
305+
try {
306+
if ( em.getTransaction().isActive() ) {
307+
log.warn( "You left an open transaction! Fix your test case. For now, we are closing it for you." );
308+
em.getTransaction().rollback();
309+
}
310+
}
311+
finally {
312+
if ( em.isOpen() ) {
313+
// as we open an EM before the test runs, it will still be open if the test uses a custom EM.
314+
// or, the person may have forgotten to close. So, do not raise a "fail", but log the fact.
315+
log.warn( "The EntityManager is not closed. Closing it." );
316+
em.close();
317+
}
318+
}
308319
}
309-
if ( em.isOpen() ) {
310-
// as we open an EM before the test runs, it will still be open if the test uses a custom EM.
311-
// or, the person may have forgotten to close. So, do not raise a "fail", but log the fact.
312-
em.close();
313-
log.warn("The EntityManager is not closed. Closing it.");
320+
catch (RuntimeException e) {
321+
log.debug( "Ignoring exception during clean up", e );
314322
}
315323
}
316324

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,8 @@ public void testNoFromClauseInSubquery(EntityManagerFactoryScope scope) {
9494
assertEquals("Jack", entities.get(0).getName());
9595
assertEquals("Black", entities.get(0).getSurName());
9696
assertEquals("John", entities.get(1).getName());
97-
assertEquals("Doe", entities.get(1).getSurName());
97+
// Trim because in some dialects, the type is CHAR(5), leading to trailing spaces
98+
assertEquals("Doe", entities.get(1).getSurName().trim());
9899
}
99100
);
100101
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ public class TypesafeNamedQueryTest {
2222
scope.inTransaction( entityManager -> {
2323
Record record1 = new Record("Hello, World!");
2424
Record record2 = new Record("Goodbye!");
25+
// make sure record2 timestamp is actually different from record1 to prevent test failure
26+
record2.timestamp = record1.timestamp.plusNanos( scope.getDialect().getFractionalSecondPrecisionInNanos() );
2527
entityManager.persist(record1);
2628
entityManager.persist(record2);
2729
String text = entityManager.createQuery(Record_._TextById_).setParameter(1, record1.id).getSingleResult();

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@
3535
import java.util.List;
3636
import java.util.function.Function;
3737

38+
import static org.hamcrest.MatcherAssert.assertThat;
39+
import static org.hamcrest.Matchers.containsString;
3840
import static org.junit.jupiter.api.Assertions.assertEquals;
3941
import static org.junit.jupiter.api.Assertions.assertNotNull;
4042
import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -369,9 +371,9 @@ void testMultiLoadSimpleIdEntityOptimisticForceIncrementLock(SessionFactoryScope
369371
}
370372

371373
private void checkStatement(int stmtCount, String lockString) {
372-
assertEquals( stmtCount,sqlStatementInspector.getSqlQueries().size() );
374+
assertEquals( stmtCount, sqlStatementInspector.getSqlQueries().size() );
373375
for ( String stmt : sqlStatementInspector.getSqlQueries() ) {
374-
assertTrue( stmt.contains( lockString ) );
376+
assertThat( stmt, containsString( lockString ) );
375377
}
376378
sqlStatementInspector.clear();
377379
}

‎hibernate-core/src/test/java/org/hibernate/orm/test/query/CteTests.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import java.util.List;
99
import java.util.function.Consumer;
1010

11+
import org.hibernate.community.dialect.FirebirdDialect;
1112
import org.hibernate.dialect.SybaseASEDialect;
1213
import org.hibernate.community.dialect.TiDBDialect;
1314
import org.hibernate.query.Query;
@@ -239,6 +240,7 @@ public void testSubquery(SessionFactoryScope scope) {
239240

240241
@Test
241242
@SkipForDialect(dialectClass = SybaseASEDialect.class, reason = "The emulation of CTEs in subqueries results in correlation in nesting level 2, which is not possible with Sybase ASE")
243+
@SkipForDialect( dialectClass = FirebirdDialect.class, reason = "Current Firebird versions treat this as an IN value list, see also https://github.com/FirebirdSQL/firebird/issues/8182" )
242244
public void testInSubquery(SessionFactoryScope scope) {
243245
scope.inTransaction( session -> {
244246
final HibernateCriteriaBuilder cb = session.getCriteriaBuilder();

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
import jakarta.persistence.criteria.ParameterExpression;
1818
import org.hibernate.dialect.CockroachDialect;
19-
import org.hibernate.community.dialect.DerbyDialect;
2019
import org.hibernate.dialect.PostgreSQLDialect;
2120
import org.hibernate.query.criteria.HibernateCriteriaBuilder;
2221
import org.hibernate.query.criteria.JpaExpression;
@@ -479,7 +478,7 @@ public void testNumericTruncFunction(SessionFactoryScope scope) {
479478

480479
@Test
481480
@JiraKey("HHH-16130")
482-
@SkipForDialect(dialectClass = DerbyDialect.class, reason = "Derby doesn't support any form of date truncation")
481+
@RequiresDialectFeature( feature = DialectFeatureChecks.SupportsDateTimeTruncation.class )
483482
public void testDateTruncFunction(SessionFactoryScope scope) {
484483
scope.inTransaction( session -> {
485484
HibernateCriteriaBuilder cb = session.getCriteriaBuilder();

0 commit comments

Comments
 (0)