From b9d4a74693b62e1d46928929c7ed04646bcef06d Mon Sep 17 00:00:00 2001 From: Andrea Boriero Date: Wed, 26 Jan 2022 16:50:29 +0100 Subject: [PATCH] Fix rendering of negated boolean expression predicate --- .../dialect/FirebirdSqlAstTranslator.java | 7 + .../dialect/CockroachSqlAstTranslator.java | 14 +- .../dialect/DB2SqlAstTranslator.java | 7 + .../dialect/DerbySqlAstTranslator.java | 7 + .../hibernate/dialect/H2SqlAstTranslator.java | 7 + .../dialect/HSQLSqlAstTranslator.java | 7 + .../dialect/MariaDBSqlAstTranslator.java | 7 + .../dialect/MySQLSqlAstTranslator.java | 7 + .../dialect/PostgreSQLSqlAstTranslator.java | 7 + .../dialect/TiDBSqlAstTranslator.java | 7 + .../SqmBooleanExpressionPredicate.java | 1 + .../sql/ast/spi/AbstractSqlAstTranslator.java | 8 +- .../jpa/compliance/CriteriaIsFalseTest.java | 121 ++++++++++++++++++ 13 files changed, 205 insertions(+), 2 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/jpa/compliance/CriteriaIsFalseTest.java diff --git a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/FirebirdSqlAstTranslator.java b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/FirebirdSqlAstTranslator.java index 1929871cc6..5b3fde63ca 100644 --- a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/FirebirdSqlAstTranslator.java +++ b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/FirebirdSqlAstTranslator.java @@ -54,7 +54,14 @@ public class FirebirdSqlAstTranslator extends AbstractS @Override public void visitBooleanExpressionPredicate(BooleanExpressionPredicate booleanExpressionPredicate) { if ( getDialect().getVersion().isSameOrAfter( 3 ) ) { + final boolean isNegated = booleanExpressionPredicate.isNegated(); + if ( isNegated ) { + appendSql( "not(" ); + } booleanExpressionPredicate.getExpression().accept( this ); + if ( isNegated ) { + appendSql( CLOSE_PARENTHESIS ); + } } else { super.visitBooleanExpressionPredicate( booleanExpressionPredicate ); diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/CockroachSqlAstTranslator.java b/hibernate-core/src/main/java/org/hibernate/dialect/CockroachSqlAstTranslator.java index 5f3b95e193..3ff46653bf 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/CockroachSqlAstTranslator.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/CockroachSqlAstTranslator.java @@ -37,7 +37,19 @@ public class CockroachSqlAstTranslator extends Abstract @Override public void visitBooleanExpressionPredicate(BooleanExpressionPredicate booleanExpressionPredicate) { - booleanExpressionPredicate.getExpression().accept( this ); + if ( booleanExpressionPredicate.isNegated() ) { + super.visitBooleanExpressionPredicate( booleanExpressionPredicate ); + } + else { + final boolean isNegated = booleanExpressionPredicate.isNegated(); + if ( isNegated ) { + appendSql( "not(" ); + } + booleanExpressionPredicate.getExpression().accept( this ); + if ( isNegated ) { + appendSql( CLOSE_PARENTHESIS ); + } + } } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/DB2SqlAstTranslator.java b/hibernate-core/src/main/java/org/hibernate/dialect/DB2SqlAstTranslator.java index fc2cb816cc..5dcae6028d 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/DB2SqlAstTranslator.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/DB2SqlAstTranslator.java @@ -52,7 +52,14 @@ public class DB2SqlAstTranslator extends AbstractSqlAst @Override public void visitBooleanExpressionPredicate(BooleanExpressionPredicate booleanExpressionPredicate) { if ( getDB2Version().isSameOrAfter( 11 ) ) { + final boolean isNegated = booleanExpressionPredicate.isNegated(); + if ( isNegated ) { + appendSql( "not(" ); + } booleanExpressionPredicate.getExpression().accept( this ); + if ( isNegated ) { + appendSql( CLOSE_PARENTHESIS ); + } } else { super.visitBooleanExpressionPredicate( booleanExpressionPredicate ); diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/DerbySqlAstTranslator.java b/hibernate-core/src/main/java/org/hibernate/dialect/DerbySqlAstTranslator.java index 5d63d24b28..7d90b881eb 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/DerbySqlAstTranslator.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/DerbySqlAstTranslator.java @@ -48,7 +48,14 @@ public class DerbySqlAstTranslator extends AbstractSqlA @Override public void visitBooleanExpressionPredicate(BooleanExpressionPredicate booleanExpressionPredicate) { + final boolean isNegated = booleanExpressionPredicate.isNegated(); + if ( isNegated ) { + appendSql( "not(" ); + } booleanExpressionPredicate.getExpression().accept( this ); + if ( isNegated ) { + appendSql( CLOSE_PARENTHESIS ); + } } // Derby does not allow CASE expressions where all result arms contain plain parameters. diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/H2SqlAstTranslator.java b/hibernate-core/src/main/java/org/hibernate/dialect/H2SqlAstTranslator.java index 049db39515..2bbba57452 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/H2SqlAstTranslator.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/H2SqlAstTranslator.java @@ -47,7 +47,14 @@ public class H2SqlAstTranslator extends AbstractSqlAstT @Override public void visitBooleanExpressionPredicate(BooleanExpressionPredicate booleanExpressionPredicate) { + final boolean isNegated = booleanExpressionPredicate.isNegated(); + if ( isNegated ) { + appendSql( "not(" ); + } booleanExpressionPredicate.getExpression().accept( this ); + if ( isNegated ) { + appendSql( CLOSE_PARENTHESIS ); + } } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/HSQLSqlAstTranslator.java b/hibernate-core/src/main/java/org/hibernate/dialect/HSQLSqlAstTranslator.java index f33e1fd66e..4e8003d365 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/HSQLSqlAstTranslator.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/HSQLSqlAstTranslator.java @@ -47,7 +47,14 @@ public class HSQLSqlAstTranslator extends AbstractSqlAs @Override public void visitBooleanExpressionPredicate(BooleanExpressionPredicate booleanExpressionPredicate) { + final boolean isNegated = booleanExpressionPredicate.isNegated(); + if ( isNegated ) { + appendSql( "not(" ); + } booleanExpressionPredicate.getExpression().accept( this ); + if ( isNegated ) { + appendSql( CLOSE_PARENTHESIS ); + } } // HSQL does not allow CASE expressions where all result arms contain plain parameters. diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/MariaDBSqlAstTranslator.java b/hibernate-core/src/main/java/org/hibernate/dialect/MariaDBSqlAstTranslator.java index f7a2771807..64d2d766e2 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/MariaDBSqlAstTranslator.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/MariaDBSqlAstTranslator.java @@ -39,7 +39,14 @@ public class MariaDBSqlAstTranslator extends AbstractSq @Override public void visitBooleanExpressionPredicate(BooleanExpressionPredicate booleanExpressionPredicate) { + final boolean isNegated = booleanExpressionPredicate.isNegated(); + if ( isNegated ) { + appendSql( "not(" ); + } booleanExpressionPredicate.getExpression().accept( this ); + if ( isNegated ) { + appendSql( CLOSE_PARENTHESIS ); + } } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/MySQLSqlAstTranslator.java b/hibernate-core/src/main/java/org/hibernate/dialect/MySQLSqlAstTranslator.java index d7dc1f2dd6..d3d5f2760b 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/MySQLSqlAstTranslator.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/MySQLSqlAstTranslator.java @@ -40,7 +40,14 @@ public class MySQLSqlAstTranslator extends AbstractSqlA @Override public void visitBooleanExpressionPredicate(BooleanExpressionPredicate booleanExpressionPredicate) { + final boolean isNegated = booleanExpressionPredicate.isNegated(); + if ( isNegated ) { + appendSql( "not(" ); + } booleanExpressionPredicate.getExpression().accept( this ); + if ( isNegated ) { + appendSql( CLOSE_PARENTHESIS ); + } } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/PostgreSQLSqlAstTranslator.java b/hibernate-core/src/main/java/org/hibernate/dialect/PostgreSQLSqlAstTranslator.java index 7042a3f915..006c4c7b14 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/PostgreSQLSqlAstTranslator.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/PostgreSQLSqlAstTranslator.java @@ -40,7 +40,14 @@ public class PostgreSQLSqlAstTranslator extends Abstrac @Override public void visitBooleanExpressionPredicate(BooleanExpressionPredicate booleanExpressionPredicate) { + final boolean isNegated = booleanExpressionPredicate.isNegated(); + if ( isNegated ) { + appendSql( "not(" ); + } booleanExpressionPredicate.getExpression().accept( this ); + if ( isNegated ) { + appendSql( CLOSE_PARENTHESIS ); + } } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/TiDBSqlAstTranslator.java b/hibernate-core/src/main/java/org/hibernate/dialect/TiDBSqlAstTranslator.java index ea0ed5e4d6..bf484074fa 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/TiDBSqlAstTranslator.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/TiDBSqlAstTranslator.java @@ -40,7 +40,14 @@ public class TiDBSqlAstTranslator extends AbstractSqlAs @Override public void visitBooleanExpressionPredicate(BooleanExpressionPredicate booleanExpressionPredicate) { + final boolean isNegated = booleanExpressionPredicate.isNegated(); + if ( isNegated ) { + appendSql( "not(" ); + } booleanExpressionPredicate.getExpression().accept( this ); + if ( isNegated ) { + appendSql( CLOSE_PARENTHESIS ); + } } protected boolean shouldEmulateFetchClause(QueryPart queryPart) { diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/predicate/SqmBooleanExpressionPredicate.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/predicate/SqmBooleanExpressionPredicate.java index c4c147ce29..25e2e8a426 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/predicate/SqmBooleanExpressionPredicate.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/predicate/SqmBooleanExpressionPredicate.java @@ -13,6 +13,7 @@ import jakarta.persistence.criteria.Expression; import org.hibernate.query.sqm.NodeBuilder; import org.hibernate.query.sqm.SemanticQueryWalker; import org.hibernate.query.sqm.tree.expression.SqmExpression; +import org.hibernate.sql.ast.tree.predicate.NegatedPredicate; /** * Represents an expression whose type is boolean, and can therefore be used as a predicate. 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 1c9947c353..034b164af3 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 @@ -4630,7 +4630,13 @@ public abstract class AbstractSqlAstTranslator implemen // Most databases do not support boolean expressions in a predicate context, so we render `expr=true` booleanExpressionPredicate.getExpression().accept( this ); appendSql( '=' ); - getDialect().appendBooleanValueString( this, true ); + if ( booleanExpressionPredicate.isNegated() ) { + getDialect().appendBooleanValueString( this, false ); + + } + else { + getDialect().appendBooleanValueString( this, true ); + } } @Override diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/compliance/CriteriaIsFalseTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/compliance/CriteriaIsFalseTest.java new file mode 100644 index 0000000000..b000eb285e --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/compliance/CriteriaIsFalseTest.java @@ -0,0 +1,121 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later + * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html + */ +package org.hibernate.orm.test.jpa.compliance; + +import java.util.List; + +import org.hibernate.testing.orm.junit.EntityManagerFactoryScope; +import org.hibernate.testing.orm.junit.Jpa; +import org.junit.jupiter.api.Test; + +import jakarta.persistence.Entity; +import jakarta.persistence.Id; +import jakarta.persistence.OneToOne; +import jakarta.persistence.Table; +import jakarta.persistence.criteria.CriteriaBuilder; +import jakarta.persistence.criteria.CriteriaQuery; +import jakarta.persistence.criteria.Root; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +@Jpa( + annotatedClasses = { + CriteriaIsFalseTest.Person.class, + CriteriaIsFalseTest.Address.class + } +) +public class CriteriaIsFalseTest { + + @Test + public void testIsFalse(EntityManagerFactoryScope scope) { + + Address validAddress = new Address( 1, "Lollard street London", true ); + Address invalidAddress = new Address( 2, "Oxfort street London", false ); + Person personWithValidAddress = new Person( 1, "Luigi", validAddress ); + Person personWithInvalidAddredd = new Person( 2, "Andrea", invalidAddress ); + + scope.inTransaction( + entityManager -> { + entityManager.persist( validAddress ); + entityManager.persist( invalidAddress ); + entityManager.persist( personWithValidAddress ); + entityManager.persist( personWithInvalidAddredd ); + } + ); + + scope.inEntityManager( + entityManager -> { + final CriteriaBuilder criteriaBuilder = entityManager.getCriteriaBuilder(); + + final CriteriaQuery query = criteriaBuilder.createQuery( Integer.class ); + final Root personRoot = query.from( Person.class ); + query.select( personRoot.get( "id" ) ); + query.where( criteriaBuilder.isFalse( personRoot.get( "address" ).get( "valid" ) ) ); + + final List ids = entityManager.createQuery( query ).getResultList(); + + assertEquals( 1, ids.size() ); + assertEquals( personWithInvalidAddredd.getId(), ids.get( 0 ) ); + } + ); + + } + + @Entity(name = "Person") + @Table(name = "PERSON_TABLE") + public static class Person { + @Id + private Integer id; + + private String name; + + @OneToOne + private Address address; + + public Person() { + } + + public Person(Integer id, String name, Address address) { + this.id = id; + this.name = name; + this.address = address; + } + + public Integer getId() { + return id; + } + + public String getName() { + return name; + } + + public Address getAddress() { + return address; + } + } + + @Entity(name = "Address") + @Table(name = "ADDRESS_TABLE") + public static class Address { + + @Id + private Integer id; + + private String street; + + private boolean valid; + + public Address() { + } + + public Address(Integer id, String street, boolean valid) { + this.id = id; + this.street = street; + this.valid = valid; + } + } +}