From 1bd0180172e17c0e9803792ca3c90fe3d875766e Mon Sep 17 00:00:00 2001 From: Christian Beikov Date: Thu, 2 Mar 2023 21:53:38 +0100 Subject: [PATCH] HHH-16182 Fix some tests for older databases and adapt assertion for boolean function --- .../mapping/basic/BooleanMappingTests.java | 25 +++++------- .../sqm/sql/BaseSqmToSqlAstConverter.java | 33 +++++++++------ .../jpa/graphs/CacheableEntityGraphTest.java | 40 +++++++++---------- 3 files changed, 50 insertions(+), 48 deletions(-) diff --git a/documentation/src/test/java/org/hibernate/userguide/mapping/basic/BooleanMappingTests.java b/documentation/src/test/java/org/hibernate/userguide/mapping/basic/BooleanMappingTests.java index c8988f63cf..54b664772b 100644 --- a/documentation/src/test/java/org/hibernate/userguide/mapping/basic/BooleanMappingTests.java +++ b/documentation/src/test/java/org/hibernate/userguide/mapping/basic/BooleanMappingTests.java @@ -10,6 +10,8 @@ import java.sql.Types; import org.hibernate.boot.model.FunctionContributions; import org.hibernate.boot.model.FunctionContributor; +import org.hibernate.dialect.AbstractHANADialect; +import org.hibernate.dialect.DB2Dialect; import org.hibernate.dialect.OracleDialect; import org.hibernate.dialect.SQLServerDialect; import org.hibernate.dialect.SybaseASEDialect; @@ -330,16 +332,7 @@ public class BooleanMappingTests { return result.intValue(); } - /** - * @implNote Skipped for dialects without support for boolean (predicate) expressions. The test - * is really about handling the SQM function reference anyway; the actual Dialect implementation - * is not standard. - */ @Test - @SkipForDialect(dialectClass = OracleDialect.class) - @SkipForDialect(dialectClass = SybaseDialect.class) - @SkipForDialect(dialectClass = SybaseASEDialect.class) - @SkipForDialect(dialectClass = SQLServerDialect.class) public void testBooleanFunctionAsPredicate(SessionFactoryScope scope) { // Not strictly relevant to boolean mappings, but test that boolean // functions work *as a* predicate after HHH-16182 @@ -351,20 +344,20 @@ public class BooleanMappingTests { } ); assertThat( statementInspector.getSqlQueries().size(), equalTo( 1 ) ); - assertThat( statementInspector.getSqlQueries().get( 0 ), containsString( "(1=1)" ) ); - assertThat( statementInspector.getSqlQueries().get( 0 ), containsString( "(2=2)" ) ); + assertThat( statementInspector.getSqlQueries().get( 0 ), containsString( "where (1=1) or (2=2)" ) ); } /** - * @implNote Skipped for dialects without support for boolean (predicate) expressions. The test - * is really about handling the SQM function reference anyway; the actual Dialect implementation - * is not standard. + * @implNote Skipped for dialects without support for comparing a boolean predicate against a boolean expressions, + * i.e. `(1=1)=true`. The test is really about handling the SQM function reference anyway; + * the actual Dialect implementation is not standard. */ @Test @SkipForDialect(dialectClass = OracleDialect.class) - @SkipForDialect(dialectClass = SybaseDialect.class) - @SkipForDialect(dialectClass = SybaseASEDialect.class) @SkipForDialect(dialectClass = SQLServerDialect.class) + @SkipForDialect(dialectClass = SybaseDialect.class, matchSubTypes = true) + @SkipForDialect(dialectClass = AbstractHANADialect.class, matchSubTypes = true) + @SkipForDialect(dialectClass = DB2Dialect.class, majorVersion = 10) public void testBooleanFunctionInPredicate(SessionFactoryScope scope) { // Not strictly relevant to boolean mappings, but test that boolean // functions work *in a* predicate after HHH-16182 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 478281b933..6d5bc29fb1 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 @@ -7058,21 +7058,30 @@ public abstract class BaseSqmToSqlAstConverter extends Base @Override public Object visitBooleanExpressionPredicate(SqmBooleanExpressionPredicate predicate) { final Expression booleanExpression = (Expression) predicate.getBooleanExpression().accept( this ); - final JdbcMapping jdbcMapping = booleanExpression.getExpressionType().getJdbcMapping( 0 ); - if ( jdbcMapping.getValueConverter() != null ) { - // handle converted booleans (yes-no, etc) - return new ComparisonPredicate( + if ( booleanExpression instanceof SelfRenderingExpression ) { + final Predicate sqlPredicate = new SelfRenderingPredicate( (SelfRenderingExpression) booleanExpression ); + if ( predicate.isNegated() ) { + return new NegatedPredicate( sqlPredicate ); + } + return sqlPredicate; + } + else { + final JdbcMapping jdbcMapping = booleanExpression.getExpressionType().getJdbcMapping( 0 ); + if ( jdbcMapping.getValueConverter() != null ) { + // handle converted booleans (yes-no, etc) + return new ComparisonPredicate( + booleanExpression, + ComparisonOperator.EQUAL, + new JdbcLiteral<>( jdbcMapping.convertToRelationalValue( !predicate.isNegated() ), jdbcMapping ) + ); + } + + return new BooleanExpressionPredicate( booleanExpression, - ComparisonOperator.EQUAL, - new JdbcLiteral<>( jdbcMapping.convertToRelationalValue( !predicate.isNegated() ), jdbcMapping ) + predicate.isNegated(), + getBooleanType() ); } - - return new BooleanExpressionPredicate( - booleanExpression, - predicate.isNegated(), - getBooleanType() - ); } @Override diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/graphs/CacheableEntityGraphTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/graphs/CacheableEntityGraphTest.java index aaeea1cb13..ee2741f52b 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/graphs/CacheableEntityGraphTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/graphs/CacheableEntityGraphTest.java @@ -23,9 +23,11 @@ import jakarta.persistence.Id; import jakarta.persistence.ManyToOne; import jakarta.persistence.OneToOne; import jakarta.persistence.Version; + import org.hibernate.annotations.Cache; import org.hibernate.annotations.CacheConcurrencyStrategy; import org.hibernate.orm.test.jpa.BaseEntityManagerFunctionalTestCase; + import org.hibernate.testing.TestForIssue; import org.junit.Test; @@ -33,7 +35,7 @@ public class CacheableEntityGraphTest extends BaseEntityManagerFunctionalTestCas @Override protected Class[] getAnnotatedClasses() { - return new Class[]{Product.class, Color.class, Tag.class}; + return new Class[] { Product.class, Color.class, Tag.class }; } @Test @@ -42,34 +44,32 @@ public class CacheableEntityGraphTest extends BaseEntityManagerFunctionalTestCas EntityManager em = getOrCreateEntityManager(); em.getTransaction().begin(); - Tag tag = new Tag(Set.of(TagType.FOO)); - em.persist(tag); + Tag tag = new Tag( Set.of( TagType.FOO ) ); + em.persist( tag ); Color color = new Color(); - em.persist(color); + em.persist( color ); - Product product = new Product(tag, color); - em.persist(product); + Product product = new Product( tag, color ); + em.persist( product ); em.getTransaction().commit(); em.clear(); - EntityGraph entityGraph = em.createEntityGraph(Product.class); - entityGraph.addAttributeNodes("tag"); + EntityGraph entityGraph = em.createEntityGraph( Product.class ); + entityGraph.addAttributeNodes( "tag" ); - em.createQuery( - "select p from org.hibernate.orm.test.jpa.graphs.CacheableEntityGraphTest$Product p", - Product.class) - .setMaxResults(2) - .setHint("jakarta.persistence.loadgraph", entityGraph) + em.createQuery( "select p from Product p", Product.class ) + .setMaxResults( 2 ) + .setHint( "jakarta.persistence.loadgraph", entityGraph ) .getSingleResult(); } - @Entity + @Entity(name = "Product") public static class Product { @Id - @GeneratedValue(strategy = GenerationType.IDENTITY) + @GeneratedValue public int id; @ManyToOne(fetch = FetchType.LAZY) @@ -87,11 +87,11 @@ public class CacheableEntityGraphTest extends BaseEntityManagerFunctionalTestCas } } - @Entity + @Entity(name = "Color") public static class Color { @Id - @GeneratedValue(strategy = GenerationType.IDENTITY) + @GeneratedValue public int id; @OneToOne(fetch = FetchType.LAZY) @@ -99,11 +99,11 @@ public class CacheableEntityGraphTest extends BaseEntityManagerFunctionalTestCas } @Cacheable - @Entity + @Entity(name = "Tag") public static class Tag { @Id - @GeneratedValue(strategy = GenerationType.IDENTITY) + @GeneratedValue public int id; @Version @@ -118,7 +118,7 @@ public class CacheableEntityGraphTest extends BaseEntityManagerFunctionalTestCas } public Tag(Set types) { - this.types.addAll(types); + this.types.addAll( types ); } }