Skip to content

Commit baefe50

Browse files
Mike Mannionbeikov
Mike Mannion
authored and
beikov
committed
HHH-19497 fixes incorrect implementation of 'negated in'
1 parent 1953ddc commit baefe50

File tree

4 files changed

+208
-9
lines changed

4 files changed

+208
-9
lines changed

‎hibernate-core/src/main/java/org/hibernate/sql/ast/spi/AbstractSqlAstTranslator.java

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6904,9 +6904,6 @@ public void visitInListPredicate(InListPredicate inListPredicate) {
69046904
}
69056905
}
69066906
else if ( !supportsRowValueConstructorSyntaxInInList() ) {
6907-
final ComparisonOperator comparisonOperator = inListPredicate.isNegated() ?
6908-
ComparisonOperator.NOT_EQUAL :
6909-
ComparisonOperator.EQUAL;
69106907
// Some DBs like Oracle support tuples only for the IN subquery predicate
69116908
if ( supportsRowValueConstructorSyntaxInInSubQuery() && dialect.supportsUnionAll() ) {
69126909
inListPredicate.getTestExpression().accept( this );
@@ -6925,17 +6922,21 @@ else if ( !supportsRowValueConstructorSyntaxInInList() ) {
69256922
appendSql( CLOSE_PARENTHESIS );
69266923
}
69276924
else {
6928-
String separator = NO_SEPARATOR;
6925+
final ComparisonOperator tupleComparisonOperator = inListPredicate.isNegated() ?
6926+
ComparisonOperator.NOT_EQUAL :
6927+
ComparisonOperator.EQUAL;
6928+
final String expressionJunction = inListPredicate.isNegated() ? " and " : " or ";
69296929
appendSql( OPEN_PARENTHESIS );
6930-
for ( Expression expression : listExpressions ) {
6931-
appendSql( separator );
6930+
String separator = NO_SEPARATOR;
6931+
for (Expression expression : listExpressions) {
6932+
appendSql(separator);
69326933
emulateTupleComparison(
69336934
lhsTuple.getExpressions(),
6934-
SqlTupleContainer.getSqlTuple( expression ).getExpressions(),
6935-
comparisonOperator,
6935+
SqlTupleContainer.getSqlTuple(expression).getExpressions(),
6936+
tupleComparisonOperator,
69366937
true
69376938
);
6938-
separator = " or ";
6939+
separator = expressionJunction;
69396940
}
69406941
appendSql( CLOSE_PARENTHESIS );
69416942
}

‎hibernate-core/src/test/java/org/hibernate/orm/test/jpa/criteria/basic/Foo.java

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
* Copyright Red Hat Inc. and Hibernate Authors
4+
*/
5+
package org.hibernate.orm.test.jpa.criteria.basic;
6+
import jakarta.persistence.Entity;
7+
import jakarta.persistence.Id;
8+
import jakarta.persistence.JoinColumn;
9+
import jakarta.persistence.JoinColumns;
10+
import jakarta.persistence.ManyToOne;
11+
12+
import java.util.Objects;
13+
14+
/**
15+
* Entity used in <code>org.hibernate.orm.test.jpa.criteria.basic.NegatedInPredicateTest</code>.
16+
*
17+
* @author Mike Mannion
18+
*/
19+
@Entity
20+
public class Foo {
21+
22+
@Id
23+
Long id;
24+
25+
@ManyToOne
26+
@JoinColumns({
27+
@JoinColumn(name = "FK_CODE", referencedColumnName = "CODE"),
28+
@JoinColumn(name = "FK_CONTEXT", referencedColumnName = "CONTEXT")
29+
})
30+
FooType fooType;
31+
32+
public Foo() {
33+
// For JPA
34+
}
35+
36+
public Foo(Long id, FooType fooType) {
37+
this.id = id;
38+
this.fooType = fooType;
39+
}
40+
41+
@Override
42+
public boolean equals(Object o) {
43+
if (o == null || getClass() != o.getClass()) return false;
44+
45+
Foo customer = (Foo) o;
46+
return Objects.equals(id, customer.id) && Objects.equals(fooType, customer.fooType);
47+
}
48+
49+
@Override
50+
public int hashCode() {
51+
int result = Objects.hashCode(id);
52+
result = 31 * result + Objects.hashCode(fooType);
53+
return result;
54+
}
55+
}

‎hibernate-core/src/test/java/org/hibernate/orm/test/jpa/criteria/basic/FooType.java

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
* Copyright Red Hat Inc. and Hibernate Authors
4+
*/
5+
package org.hibernate.orm.test.jpa.criteria.basic;
6+
import jakarta.persistence.Column;
7+
import jakarta.persistence.Entity;
8+
import jakarta.persistence.Id;
9+
10+
/**
11+
* Entity used in <code>org.hibernate.orm.test.jpa.criteria.basic.NegatedInPredicateTest</code>.
12+
*
13+
* @author Mike Mannion
14+
*/
15+
@Entity
16+
public class FooType {
17+
@Id
18+
@Column(name = "CODE")
19+
String code;
20+
21+
@Column(name = "CONTEXT")
22+
String context;
23+
24+
public FooType() {
25+
// For JPA
26+
}
27+
28+
FooType(String code, String context) {
29+
this.code = code;
30+
this.context = context;
31+
}
32+
}

‎hibernate-core/src/test/java/org/hibernate/orm/test/jpa/criteria/basic/NegatedInPredicateTest.java

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
* Copyright Red Hat Inc. and Hibernate Authors
4+
*/
5+
package org.hibernate.orm.test.jpa.criteria.basic;
6+
7+
import jakarta.persistence.criteria.CriteriaBuilder;
8+
import jakarta.persistence.criteria.CriteriaQuery;
9+
import jakarta.persistence.criteria.Root;
10+
import org.hibernate.testing.orm.junit.DomainModel;
11+
import org.hibernate.testing.orm.junit.JiraKey;
12+
import org.hibernate.testing.orm.junit.SessionFactory;
13+
import org.hibernate.testing.orm.junit.SessionFactoryScope;
14+
import org.junit.jupiter.api.AfterEach;
15+
import org.junit.jupiter.api.BeforeEach;
16+
import org.junit.jupiter.api.Test;
17+
18+
import java.util.List;
19+
20+
import static org.assertj.core.api.Assertions.assertThat;
21+
22+
23+
/**
24+
* Add test which composes not with in. Test introduced after discovery the negated in
25+
* fails under dialects without record-level construction, such as DB2.
26+
*
27+
* @author Mike Mannion
28+
*/
29+
@JiraKey(value = "HHH-19497")
30+
@DomainModel(
31+
annotatedClasses = {Foo.class, FooType.class}
32+
)
33+
@SessionFactory
34+
class NegatedInPredicateTest {
35+
36+
public static final FooType FOO_TYPE1 = new FooType( "ft1", "ctx1" );
37+
public static final FooType FOO_TYPE2 = new FooType( "ft2", "ctx1" );
38+
39+
@BeforeEach
40+
void setup(SessionFactoryScope scope) {
41+
scope.inTransaction(
42+
em -> {
43+
em.persist( FOO_TYPE1 );
44+
em.persist( FOO_TYPE2 );
45+
46+
Foo foo1 = new Foo( 1L, FOO_TYPE1 );
47+
Foo foo2 = new Foo( 2L, FOO_TYPE1 );
48+
Foo foo3 = new Foo( 3L, FOO_TYPE2 );
49+
50+
em.persist( foo1 );
51+
em.persist( foo2 );
52+
em.persist( foo3 );
53+
}
54+
);
55+
}
56+
57+
@AfterEach
58+
void tearDown(SessionFactoryScope scope) {
59+
scope.inTransaction(
60+
em -> {
61+
em.createQuery( "delete from Foo" ).executeUpdate();
62+
em.createQuery( "delete from FooType" ).executeUpdate();
63+
}
64+
);
65+
}
66+
67+
@Test
68+
void testSanity(SessionFactoryScope scope) {
69+
scope.inTransaction(
70+
em -> {
71+
CriteriaBuilder cb = em.getCriteriaBuilder();
72+
CriteriaQuery<Foo> cq = cb.createQuery( Foo.class );
73+
Root<Foo> root = cq.from( Foo.class );
74+
assertThat( em.createQuery( cq.select( root ) ).getResultList() )
75+
.hasSize( 3 );
76+
}
77+
);
78+
}
79+
80+
@Test
81+
void testNegatedPredicate(SessionFactoryScope scope) {
82+
scope.inTransaction(
83+
em -> {
84+
CriteriaBuilder cb = em.getCriteriaBuilder();
85+
CriteriaQuery<Foo> cq = cb.createQuery( Foo.class );
86+
Root<Foo> root = cq.from( Foo.class );
87+
cq.select( root )
88+
.where( cb.not( root.get( "fooType" ).in( List.of( FOO_TYPE1, FOO_TYPE2 ) ) ) );
89+
assertThat( em.createQuery( cq ).getResultList() )
90+
.isEmpty();
91+
}
92+
);
93+
}
94+
95+
@Test
96+
void testNonNegatedInPredicate(SessionFactoryScope scope) {
97+
scope.inTransaction(
98+
em -> {
99+
CriteriaBuilder cb = em.getCriteriaBuilder();
100+
CriteriaQuery<Foo> cq = cb.createQuery( Foo.class );
101+
Root<Foo> root = cq.from( Foo.class );
102+
cq.select( root )
103+
.where( root.get( "fooType" ).in( List.of( FOO_TYPE1, FOO_TYPE2 ) ) );
104+
assertThat( em.createQuery( cq ).getResultList() )
105+
.hasSize( 3 );
106+
107+
}
108+
);
109+
}
110+
111+
}

0 commit comments

Comments
 (0)