Skip to content

Commit 8412793

Browse files
thomasrebelezabetak
authored andcommitted
[CALCITE-7291] Verify that the same exception is thrown for the original and simplified expression in RexSimplify#verify
1 parent e8ac01c commit 8412793

File tree

2 files changed

+37
-13
lines changed

2 files changed

+37
-13
lines changed

core/src/main/java/org/apache/calcite/rex/RexSimplify.java

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -440,7 +440,7 @@ private RexNode simplifyArithmetic(RexCall e) {
440440
assert e.getOperands().size() == 2;
441441

442442
switch (e.getKind()) {
443-
// These simplifications are safe for both checked and unchecked arithemtic.
443+
// These simplifications are safe for both checked and unchecked arithmetic.
444444
case PLUS:
445445
case CHECKED_PLUS:
446446
return simplifyPlus(e);
@@ -454,7 +454,7 @@ private RexNode simplifyArithmetic(RexCall e) {
454454
case CHECKED_DIVIDE:
455455
return simplifyDivide(e);
456456
default:
457-
throw new IllegalArgumentException("Unsupported arithmeitc operation " + e.getKind());
457+
throw new IllegalArgumentException("Unsupported arithmetic operation " + e.getKind());
458458
}
459459
}
460460

@@ -2309,6 +2309,20 @@ private RexNode simplifyOrs(List<RexNode> terms, RexUnknownAs unknownAs) {
23092309
return RexUtil.composeDisjunction(rexBuilder, terms);
23102310
}
23112311

2312+
private Pair<Comparable, RuntimeException> evaluate(RexNode e, Map<RexNode, Comparable> map) {
2313+
Comparable c = null;
2314+
RuntimeException ex = null;
2315+
try {
2316+
c = RexInterpreter.evaluate(e, map);
2317+
} catch (RuntimeException exception) {
2318+
ex = exception;
2319+
}
2320+
if (c == null && ex == null) {
2321+
throw new AssertionError("interpreter returned null for " + e);
2322+
}
2323+
return Pair.of(c, ex);
2324+
}
2325+
23122326
private void verify(RexNode before, RexNode simplified, RexUnknownAs unknownAs) {
23132327
if (simplified.isAlwaysFalse()
23142328
&& before.isAlwaysTrue()) {
@@ -2339,14 +2353,28 @@ private void verify(RexNode before, RexNode simplified, RexUnknownAs unknownAs)
23392353
continue assignment_loop;
23402354
}
23412355
}
2342-
Comparable v0 = RexInterpreter.evaluate(foo0.e, map);
2343-
if (v0 == null) {
2344-
throw new AssertionError("interpreter returned null for " + foo0.e);
2345-
}
2346-
Comparable v1 = RexInterpreter.evaluate(foo1.e, map);
2347-
if (v1 == null) {
2348-
throw new AssertionError("interpreter returned null for " + foo1.e);
2356+
Pair<Comparable, RuntimeException> p0 = evaluate(foo0.e, map);
2357+
Pair<Comparable, RuntimeException> p1 = evaluate(foo1.e, map);
2358+
if (p0.right != null || p1.right != null) {
2359+
if (p0.right == null || p1.right == null) {
2360+
throw Util.first(p0.right, p1.right);
2361+
}
2362+
if (!p0.right.getClass().equals(p1.right.getClass())) {
2363+
AssertionError error = new AssertionError("exception class does not match");
2364+
error.addSuppressed(p0.right);
2365+
error.addSuppressed(p1.right);
2366+
throw error;
2367+
}
2368+
if (!java.util.Objects.equals(p0.right.getMessage(), p1.right.getMessage())) {
2369+
AssertionError error = new AssertionError("exception message does not match");
2370+
error.addSuppressed(p0.right);
2371+
error.addSuppressed(p1.right);
2372+
throw error;
2373+
}
2374+
continue;
23492375
}
2376+
Comparable v0 = p0.left;
2377+
Comparable v1 = p1.left;
23502378
if (before.getType().getSqlTypeName() == SqlTypeName.BOOLEAN) {
23512379
switch (unknownAs) {
23522380
case FALSE:

core/src/test/java/org/apache/calcite/rex/RexProgramTest.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2673,7 +2673,6 @@ trueLiteral, literal(1),
26732673

26742674
@Test void testSimplifyCaseCompactionDiv() {
26752675
// FIXME: RexInterpreter currently evaluates children beforehand.
2676-
simplify = simplify.withParanoid(false);
26772676
RexNode caseNode =
26782677
case_(vBool(0), vInt(0),
26792678
eq(div(literal(3), vIntNotNull()), literal(11)), vInt(0),
@@ -2685,7 +2684,6 @@ trueLiteral, literal(1),
26852684
/** Tests a CASE value branch that contains division. */
26862685
@Test void testSimplifyCaseDiv1() {
26872686
// FIXME: RexInterpreter currently evaluates children beforehand.
2688-
simplify = simplify.withParanoid(false);
26892687
RexNode caseNode =
26902688
case_(ne(vIntNotNull(), literal(0)),
26912689
eq(div(literal(3), vIntNotNull()), literal(11)),
@@ -2696,7 +2694,6 @@ trueLiteral, literal(1),
26962694
/** Tests a CASE condition that contains division. */
26972695
@Test void testSimplifyCaseDiv2() {
26982696
// FIXME: RexInterpreter currently evaluates children beforehand.
2699-
simplify = simplify.withParanoid(false);
27002697
RexNode caseNode =
27012698
case_(eq(vIntNotNull(), literal(0)), trueLiteral,
27022699
gt(div(literal(3), vIntNotNull()), literal(1)), trueLiteral,
@@ -2718,7 +2715,6 @@ trueLiteral, literal(1),
27182715
// null + (a/0)/4
27192716
// ==>
27202717
// null + (a/0)/4
2721-
simplify = simplify.withParanoid(false);
27222718
RexNode divideNode0 = plus(nullInt, div(div(vIntNotNull(), literal(0)), literal(4)));
27232719
checkSimplifyUnchanged(divideNode0);
27242720
// null + a/4

0 commit comments

Comments
 (0)