Skip to content

Commit f35c972

Browse files
authored
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
1 parent 644d77c commit f35c972

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
@@ -671,6 +671,8 @@ public void testSimplifyCaseConditionsFoldWhenFalse() {
671671
new IfConditional(EMPTY, new Equals(EMPTY, TWO, ONE), Literal.of(EMPTY, "bar2")),
672672
new IfConditional(EMPTY, new GreaterThan(EMPTY, getFieldAttribute(), ONE), Literal.of(EMPTY, "foo2")),
673673
Literal.of(EMPTY, "default")));
674+
assertFalse(c.foldable());
675+
674676
Expression e = new SimplifyCase().rule(c);
675677
assertEquals(Case.class, e.getClass());
676678
c = (Case) e;
@@ -696,13 +698,15 @@ public void testSimplifyCaseConditionsFoldWhenTrue() {
696698
// ELSE 'default'
697699
// END
698700

699-
SimplifyCase rule = new SimplifyCase();
700701
Case c = new Case(EMPTY, Arrays.asList(
701702
new IfConditional(EMPTY, new Equals(EMPTY, getFieldAttribute(), ONE), Literal.of(EMPTY, "foo1")),
702703
new IfConditional(EMPTY, new Equals(EMPTY, ONE, ONE), Literal.of(EMPTY, "bar1")),
703704
new IfConditional(EMPTY, new Equals(EMPTY, TWO, ONE), Literal.of(EMPTY, "bar2")),
704705
new IfConditional(EMPTY, new GreaterThan(EMPTY, getFieldAttribute(), ONE), Literal.of(EMPTY, "foo2")),
705706
Literal.of(EMPTY, "default")));
707+
assertFalse(c.foldable());
708+
709+
SimplifyCase rule = new SimplifyCase();
706710
Expression e = rule.rule(c);
707711
assertEquals(Case.class, e.getClass());
708712
c = (Case) e;
@@ -713,7 +717,7 @@ public void testSimplifyCaseConditionsFoldWhenTrue() {
713717
assertEquals(TypeResolution.TYPE_RESOLVED, c.typeResolved());
714718
}
715719

716-
public void testSimplifyCaseConditionsFoldCompletely() {
720+
public void testSimplifyCaseConditionsFoldCompletely_FoldableElse() {
717721
// CASE WHEN 1 = 2 THEN 'foo1'
718722
// WHEN 1 = 1 THEN 'foo2'
719723
// ELSE 'default'
@@ -723,11 +727,13 @@ public void testSimplifyCaseConditionsFoldCompletely() {
723727
//
724728
// 'foo2'
725729

726-
SimplifyCase rule = new SimplifyCase();
727730
Case c = new Case(EMPTY, Arrays.asList(
728731
new IfConditional(EMPTY, new Equals(EMPTY, ONE, TWO), Literal.of(EMPTY, "foo1")),
729732
new IfConditional(EMPTY, new Equals(EMPTY, ONE, ONE), Literal.of(EMPTY, "foo2")),
730733
Literal.of(EMPTY, "default")));
734+
assertFalse(c.foldable());
735+
736+
SimplifyCase rule = new SimplifyCase();
731737
Expression e = rule.rule(c);
732738
assertEquals(Case.class, e.getClass());
733739
c = (Case) e;
@@ -738,9 +744,34 @@ public void testSimplifyCaseConditionsFoldCompletely() {
738744
assertEquals(TypeResolution.TYPE_RESOLVED, c.typeResolved());
739745
}
740746

741-
public void testSimplifyIif_ConditionTrue() {
747+
public void testSimplifyCaseConditionsFoldCompletely_NonFoldableElse() {
748+
// CASE WHEN 1 = 2 THEN 'foo1'
749+
// ELSE myField
750+
// END
751+
//
752+
// ==>
753+
//
754+
// myField (non-foldable)
755+
756+
Case c = new Case(EMPTY, Arrays.asList(
757+
new IfConditional(EMPTY, new Equals(EMPTY, ONE, TWO), Literal.of(EMPTY, "foo1")),
758+
getFieldAttribute("myField")));
759+
assertFalse(c.foldable());
760+
761+
SimplifyCase rule = new SimplifyCase();
762+
Expression e = rule.rule(c);
763+
assertEquals(Case.class, e.getClass());
764+
c = (Case) e;
765+
assertEquals(0, c.conditions().size());
766+
assertFalse(c.foldable());
767+
assertEquals("myField", Expressions.name(c.elseResult()));
768+
}
769+
770+
public void testSimplifyIif_ConditionTrue_FoldableResult() {
742771
SimplifyCase rule = new SimplifyCase();
743772
Iif iif = new Iif(EMPTY, new Equals(EMPTY, ONE, ONE), Literal.of(EMPTY, "foo"), Literal.of(EMPTY, "bar"));
773+
assertTrue(iif.foldable());
774+
744775
Expression e = rule.rule(iif);
745776
assertEquals(Iif.class, e.getClass());
746777
iif = (Iif) e;
@@ -750,9 +781,26 @@ public void testSimplifyIif_ConditionTrue() {
750781
assertEquals(TypeResolution.TYPE_RESOLVED, iif.typeResolved());
751782
}
752783

753-
public void testSimplifyIif_ConditionFalse() {
784+
public void testSimplifyIif_ConditionTrue_NonFoldableResult() {
785+
SimplifyCase rule = new SimplifyCase();
786+
Iif iif = new Iif(EMPTY, new Equals(EMPTY, ONE, ONE), getFieldAttribute("myField"), Literal.of(EMPTY, "bar"));
787+
assertFalse(iif.foldable());
788+
789+
Expression e = rule.rule(iif);
790+
assertEquals(Iif.class, e.getClass());
791+
iif = (Iif) e;
792+
assertEquals(1, iif.conditions().size());
793+
assertFalse(iif.foldable());
794+
assertTrue(iif.conditions().get(0).condition().foldable());
795+
assertEquals(Boolean.TRUE, iif.conditions().get(0).condition().fold());
796+
assertEquals("myField", Expressions.name(iif.conditions().get(0).result()));
797+
}
798+
799+
public void testSimplifyIif_ConditionFalse_FoldableResult() {
754800
SimplifyCase rule = new SimplifyCase();
755801
Iif iif = new Iif(EMPTY, new Equals(EMPTY, ONE, TWO), Literal.of(EMPTY, "foo"), Literal.of(EMPTY, "bar"));
802+
assertTrue(iif.foldable());
803+
756804
Expression e = rule.rule(iif);
757805
assertEquals(Iif.class, e.getClass());
758806
iif = (Iif) e;
@@ -762,6 +810,19 @@ public void testSimplifyIif_ConditionFalse() {
762810
assertEquals(TypeResolution.TYPE_RESOLVED, iif.typeResolved());
763811
}
764812

813+
public void testSimplifyIif_ConditionFalse_NonFoldableResult() {
814+
SimplifyCase rule = new SimplifyCase();
815+
Iif iif = new Iif(EMPTY, new Equals(EMPTY, ONE, TWO), Literal.of(EMPTY, "foo"), getFieldAttribute("myField"));
816+
assertFalse(iif.foldable());
817+
818+
Expression e = rule.rule(iif);
819+
assertEquals(Iif.class, e.getClass());
820+
iif = (Iif) e;
821+
assertEquals(0, iif.conditions().size());
822+
assertFalse(iif.foldable());
823+
assertEquals("myField", Expressions.name(iif.elseResult()));
824+
}
825+
765826
//
766827
// Logical simplifications
767828
//

0 commit comments

Comments
 (0)