Skip to content

Commit ef98596

Browse files
l2yujwmp911de
authored andcommitted
Refactor JPQL predicate composition by flattening AND/OR.
Signed-off-by: l2yuPa <jeungwon28@gmail.com> Closes #4195
1 parent 45e76d4 commit ef98596

File tree

2 files changed

+118
-9
lines changed

2 files changed

+118
-9
lines changed

spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpqlQueryBuilder.java

Lines changed: 82 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import java.util.Map;
2828
import java.util.Objects;
2929
import java.util.function.Supplier;
30+
import java.util.stream.Collectors;
3031

3132
import org.jspecify.annotations.Nullable;
3233

@@ -44,6 +45,7 @@
4445
*
4546
* @author Mark Paluch
4647
* @author Choi Wang Gyu
48+
* @author Jeongwon Ryu
4749
* @since 4.0
4850
*/
4951
@SuppressWarnings("JavadocDeclaration")
@@ -710,7 +712,23 @@ public interface Predicate extends Renderable {
710712
@Contract("_ -> new")
711713
@CheckReturnValue
712714
default Predicate or(Predicate other) {
713-
return new OrPredicate(this, other);
715+
716+
// Flatten OR composition: Or([a,b]).or(c) -> Or([a,b,c])
717+
List<Predicate> parts = new ArrayList<>();
718+
719+
if (this instanceof OrPredicate op) {
720+
parts.addAll(op.parts());
721+
} else {
722+
parts.add(this);
723+
}
724+
725+
if (other instanceof OrPredicate op) {
726+
parts.addAll(op.parts());
727+
} else {
728+
parts.add(other);
729+
}
730+
731+
return new OrPredicate(parts);
714732
}
715733

716734
/**
@@ -721,8 +739,27 @@ default Predicate or(Predicate other) {
721739
*/
722740
@Contract("_ -> new")
723741
@CheckReturnValue
724-
default Predicate and(Predicate other) { // don't like the structuring of this and the nest() thing
725-
return new AndPredicate(this, other);
742+
default Predicate and(Predicate other) {
743+
744+
// don't like the structuring of this and the nest() thing
745+
//
746+
// Flatten AND composition to avoid deep nesting:
747+
// And([a,b]).and(c) -> And([a,b,c])
748+
List<Predicate> parts = new ArrayList<>();
749+
750+
if (this instanceof AndPredicate ap) {
751+
parts.addAll(ap.parts());
752+
} else {
753+
parts.add(this);
754+
}
755+
756+
if (other instanceof AndPredicate ap) {
757+
parts.addAll(ap.parts());
758+
} else {
759+
parts.add(other);
760+
}
761+
762+
return new AndPredicate(parts);
726763
}
727764

728765
/**
@@ -1526,32 +1563,68 @@ private static boolean hasParenthesis(String str) {
15261563

15271564
}
15281565

1529-
record AndPredicate(Predicate left, Predicate right) implements Predicate {
1566+
record AndPredicate(List<Predicate> parts) implements Predicate {
1567+
1568+
AndPredicate {
1569+
1570+
Assert.notEmpty(parts, "Predicate parts must not be empty");
1571+
1572+
// Flatten nested AND predicates to keep the internal representation normalized.
1573+
List<Predicate> flattened = new ArrayList<>(parts.size());
1574+
for (Predicate p : parts) {
1575+
if (p instanceof AndPredicate ap) {
1576+
flattened.addAll(ap.parts());
1577+
} else {
1578+
flattened.add(p);
1579+
}
1580+
}
1581+
1582+
parts = List.copyOf(flattened);
1583+
}
15301584

15311585
@Override
15321586
public String render(RenderContext context) {
1533-
return "%s AND %s".formatted(left.render(context), right.render(context));
1587+
return parts.stream()
1588+
.map(p -> p.render(context))
1589+
.collect(Collectors.joining(" AND "));
15341590
}
15351591

15361592
@Override
15371593
public String toString() {
15381594
return render(RenderContext.EMPTY);
15391595
}
1540-
15411596
}
15421597

1543-
record OrPredicate(Predicate left, Predicate right) implements Predicate {
1598+
record OrPredicate(List<Predicate> parts) implements Predicate {
1599+
1600+
OrPredicate {
1601+
1602+
Assert.notEmpty(parts, "Predicate parts must not be empty");
1603+
1604+
// Flatten nested OR predicates to keep the internal representation normalized.
1605+
List<Predicate> flattened = new ArrayList<>(parts.size());
1606+
for (Predicate p : parts) {
1607+
if (p instanceof OrPredicate op) {
1608+
flattened.addAll(op.parts());
1609+
} else {
1610+
flattened.add(p);
1611+
}
1612+
}
1613+
1614+
parts = List.copyOf(flattened);
1615+
}
15441616

15451617
@Override
15461618
public String render(RenderContext context) {
1547-
return "%s OR %s".formatted(left.render(context), right.render(context));
1619+
return parts.stream()
1620+
.map(p -> p.render(context))
1621+
.collect(Collectors.joining(" OR "));
15481622
}
15491623

15501624
@Override
15511625
public String toString() {
15521626
return render(RenderContext.EMPTY);
15531627
}
1554-
15551628
}
15561629

15571630
record NestedPredicate(Predicate delegate) implements Predicate {

spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpqlQueryBuilderUnitTests.java

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
* @author Christoph Strobl
4646
* @author Mark Paluch
4747
* @author Choi Wang Gyu
48+
* @author Jeongwon Ryu
4849
*/
4950
class JpqlQueryBuilderUnitTests {
5051

@@ -180,6 +181,41 @@ void inPredicateWithNestedExpression() {
180181
.isEqualTo("o.country IN (SELECT c.code FROM Country c WHERE c.active = true)");
181182
}
182183

184+
@Test
185+
void chainedAndRendersWithoutExtraParentheses() {
186+
187+
Entity entity = entity(LineItem.class);
188+
189+
Join p = innerJoin(entity, "product");
190+
Join p0 = innerJoin(entity, "product2");
191+
192+
PathAndOrigin aPath = path(p, "name");
193+
PathAndOrigin bPath = path(p0, "name");
194+
195+
Predicate a = where(aPath).eq(literal("ex30"));
196+
Predicate b = where(bPath).eq(literal("ex40"));
197+
Predicate c = where(aPath).isNotNull();
198+
199+
String fragment = a.and(b).and(c).render(ctx(entity));
200+
201+
assertThat(fragment).isEqualTo("p.name = 'ex30' AND p_0.name = 'ex40' AND p.name IS NOT NULL");
202+
}
203+
204+
@Test
205+
void chainedOrRendersWithoutExtraParentheses() {
206+
207+
Entity entity = entity(Order.class);
208+
PathAndOrigin id = path(entity, "id");
209+
210+
Predicate a = where(id).eq(literal("1"));
211+
Predicate b = where(id).eq(literal("2"));
212+
Predicate c = where(id).eq(literal("3"));
213+
214+
String fragment = a.or(b).or(c).render(ctx(entity));
215+
216+
assertThat(fragment).isEqualTo("o.id = '1' OR o.id = '2' OR o.id = '3'");
217+
}
218+
183219
@Test // GH-3588
184220
void selectRendering() {
185221

0 commit comments

Comments
 (0)