Skip to content

Commit 3dc6d58

Browse files
committed
SQL: Fix issue with folding of CASE/IIF (#49449)
Add extra checks to prevent ConstantFolding rule to try to fold the CASE/IIF functions early before the SimplifyCase rule gets applied. Fixes: #49387 (cherry picked from commit f35c972)
1 parent 5a4ba4f commit 3dc6d58

File tree

2 files changed

+78
-8
lines changed
  • x-pack/plugin/sql/src
    • main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional
    • test/java/org/elasticsearch/xpack/sql/optimizer

2 files changed

+78
-8
lines changed

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/Case.java

+12-3
Original file line numberDiff line numberDiff line change
@@ -116,12 +116,21 @@ protected TypeResolution resolveType() {
116116

117117
/**
118118
* All foldable conditions that fold to FALSE should have
119-
* been removed by the {@link Optimizer}.
119+
* been removed by the {@link Optimizer}#SimplifyCase.
120120
*/
121121
@Override
122122
public boolean foldable() {
123-
return (conditions.isEmpty() && elseResult.foldable()) ||
124-
(conditions.size() == 1 && conditions.get(0).condition().foldable() && conditions.get(0).result().foldable());
123+
if (conditions.isEmpty() && elseResult.foldable()) {
124+
return true;
125+
}
126+
if (conditions.size() == 1 && conditions.get(0).condition().foldable()) {
127+
if (conditions.get(0).condition().fold() == Boolean.TRUE) {
128+
return conditions().get(0).result().foldable();
129+
} else {
130+
return elseResult().foldable();
131+
}
132+
}
133+
return false;
125134
}
126135

127136
@Override

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/optimizer/OptimizerTests.java

+66-5
Original file line numberDiff line numberDiff line change
@@ -639,6 +639,8 @@ public void testSimplifyCaseConditionsFoldWhenFalse() {
639639
new IfConditional(EMPTY, new Equals(EMPTY, TWO, ONE), Literal.of(EMPTY, "bar2")),
640640
new IfConditional(EMPTY, new GreaterThan(EMPTY, getFieldAttribute(), ONE), Literal.of(EMPTY, "foo2")),
641641
Literal.of(EMPTY, "default")));
642+
assertFalse(c.foldable());
643+
642644
Expression e = new SimplifyCase().rule(c);
643645
assertEquals(Case.class, e.getClass());
644646
c = (Case) e;
@@ -664,13 +666,15 @@ public void testSimplifyCaseConditionsFoldWhenTrue() {
664666
// ELSE 'default'
665667
// END
666668

667-
SimplifyCase rule = new SimplifyCase();
668669
Case c = new Case(EMPTY, Arrays.asList(
669670
new IfConditional(EMPTY, new Equals(EMPTY, getFieldAttribute(), ONE), Literal.of(EMPTY, "foo1")),
670671
new IfConditional(EMPTY, new Equals(EMPTY, ONE, ONE), Literal.of(EMPTY, "bar1")),
671672
new IfConditional(EMPTY, new Equals(EMPTY, TWO, ONE), Literal.of(EMPTY, "bar2")),
672673
new IfConditional(EMPTY, new GreaterThan(EMPTY, getFieldAttribute(), ONE), Literal.of(EMPTY, "foo2")),
673674
Literal.of(EMPTY, "default")));
675+
assertFalse(c.foldable());
676+
677+
SimplifyCase rule = new SimplifyCase();
674678
Expression e = rule.rule(c);
675679
assertEquals(Case.class, e.getClass());
676680
c = (Case) e;
@@ -681,7 +685,7 @@ public void testSimplifyCaseConditionsFoldWhenTrue() {
681685
assertEquals(TypeResolution.TYPE_RESOLVED, c.typeResolved());
682686
}
683687

684-
public void testSimplifyCaseConditionsFoldCompletely() {
688+
public void testSimplifyCaseConditionsFoldCompletely_FoldableElse() {
685689
// CASE WHEN 1 = 2 THEN 'foo1'
686690
// WHEN 1 = 1 THEN 'foo2'
687691
// ELSE 'default'
@@ -691,11 +695,13 @@ public void testSimplifyCaseConditionsFoldCompletely() {
691695
//
692696
// 'foo2'
693697

694-
SimplifyCase rule = new SimplifyCase();
695698
Case c = new Case(EMPTY, Arrays.asList(
696699
new IfConditional(EMPTY, new Equals(EMPTY, ONE, TWO), Literal.of(EMPTY, "foo1")),
697700
new IfConditional(EMPTY, new Equals(EMPTY, ONE, ONE), Literal.of(EMPTY, "foo2")),
698701
Literal.of(EMPTY, "default")));
702+
assertFalse(c.foldable());
703+
704+
SimplifyCase rule = new SimplifyCase();
699705
Expression e = rule.rule(c);
700706
assertEquals(Case.class, e.getClass());
701707
c = (Case) e;
@@ -706,9 +712,34 @@ public void testSimplifyCaseConditionsFoldCompletely() {
706712
assertEquals(TypeResolution.TYPE_RESOLVED, c.typeResolved());
707713
}
708714

709-
public void testSimplifyIif_ConditionTrue() {
715+
public void testSimplifyCaseConditionsFoldCompletely_NonFoldableElse() {
716+
// CASE WHEN 1 = 2 THEN 'foo1'
717+
// ELSE myField
718+
// END
719+
//
720+
// ==>
721+
//
722+
// myField (non-foldable)
723+
724+
Case c = new Case(EMPTY, Arrays.asList(
725+
new IfConditional(EMPTY, new Equals(EMPTY, ONE, TWO), Literal.of(EMPTY, "foo1")),
726+
getFieldAttribute("myField")));
727+
assertFalse(c.foldable());
728+
729+
SimplifyCase rule = new SimplifyCase();
730+
Expression e = rule.rule(c);
731+
assertEquals(Case.class, e.getClass());
732+
c = (Case) e;
733+
assertEquals(0, c.conditions().size());
734+
assertFalse(c.foldable());
735+
assertEquals("myField", Expressions.name(c.elseResult()));
736+
}
737+
738+
public void testSimplifyIif_ConditionTrue_FoldableResult() {
710739
SimplifyCase rule = new SimplifyCase();
711740
Iif iif = new Iif(EMPTY, new Equals(EMPTY, ONE, ONE), Literal.of(EMPTY, "foo"), Literal.of(EMPTY, "bar"));
741+
assertTrue(iif.foldable());
742+
712743
Expression e = rule.rule(iif);
713744
assertEquals(Iif.class, e.getClass());
714745
iif = (Iif) e;
@@ -718,9 +749,26 @@ public void testSimplifyIif_ConditionTrue() {
718749
assertEquals(TypeResolution.TYPE_RESOLVED, iif.typeResolved());
719750
}
720751

721-
public void testSimplifyIif_ConditionFalse() {
752+
public void testSimplifyIif_ConditionTrue_NonFoldableResult() {
753+
SimplifyCase rule = new SimplifyCase();
754+
Iif iif = new Iif(EMPTY, new Equals(EMPTY, ONE, ONE), getFieldAttribute("myField"), Literal.of(EMPTY, "bar"));
755+
assertFalse(iif.foldable());
756+
757+
Expression e = rule.rule(iif);
758+
assertEquals(Iif.class, e.getClass());
759+
iif = (Iif) e;
760+
assertEquals(1, iif.conditions().size());
761+
assertFalse(iif.foldable());
762+
assertTrue(iif.conditions().get(0).condition().foldable());
763+
assertEquals(Boolean.TRUE, iif.conditions().get(0).condition().fold());
764+
assertEquals("myField", Expressions.name(iif.conditions().get(0).result()));
765+
}
766+
767+
public void testSimplifyIif_ConditionFalse_FoldableResult() {
722768
SimplifyCase rule = new SimplifyCase();
723769
Iif iif = new Iif(EMPTY, new Equals(EMPTY, ONE, TWO), Literal.of(EMPTY, "foo"), Literal.of(EMPTY, "bar"));
770+
assertTrue(iif.foldable());
771+
724772
Expression e = rule.rule(iif);
725773
assertEquals(Iif.class, e.getClass());
726774
iif = (Iif) e;
@@ -730,6 +778,19 @@ public void testSimplifyIif_ConditionFalse() {
730778
assertEquals(TypeResolution.TYPE_RESOLVED, iif.typeResolved());
731779
}
732780

781+
public void testSimplifyIif_ConditionFalse_NonFoldableResult() {
782+
SimplifyCase rule = new SimplifyCase();
783+
Iif iif = new Iif(EMPTY, new Equals(EMPTY, ONE, TWO), Literal.of(EMPTY, "foo"), getFieldAttribute("myField"));
784+
assertFalse(iif.foldable());
785+
786+
Expression e = rule.rule(iif);
787+
assertEquals(Iif.class, e.getClass());
788+
iif = (Iif) e;
789+
assertEquals(0, iif.conditions().size());
790+
assertFalse(iif.foldable());
791+
assertEquals("myField", Expressions.name(iif.elseResult()));
792+
}
793+
733794
//
734795
// Logical simplifications
735796
//

0 commit comments

Comments
 (0)