vlsi commented on a change in pull request #1610: [CALCITE-3525] RexSimplify:
eliminate redundant rex calls in OR
URL: https://github.com/apache/calcite/pull/1610#discussion_r352613198
##########
File path: core/src/test/java/org/apache/calcite/test/RexProgramTest.java
##########
@@ -1738,6 +1738,20 @@ private void checkExponentialCnf(int n) {
"OR(null, <>(0, ?0.int0))",
"<>(0, ?0.int0)",
"true");
+
+ // (a=1 or a=2 or (arr[1]>4 and arr[1]<3 and a=3)) => a=1 or a=2
+ final RelDataType intArrayType = typeFactory.createArrayType(intType, -1);
+ final RexInputRef ref0 = rexBuilder.makeInputRef(intType, 0);
+ final RexInputRef ref3 = rexBuilder.makeInputRef(intArrayType, 3);
+ final RexCall itm1 = (RexCall) rexBuilder.makeCall(intType,
SqlStdOperatorTable.ITEM,
+ ImmutableList.of(ref3, literal1));
+ simplify = this.simplify.withParanoid(false);
+ checkSimplifyFilter(
+ or(eq(ref0, literal1), eq(ref0, literal2),
Review comment:
@ihuzenko , one test case per method is OK.
>And refactoring of tests is out of scope of this PR.
You add a single test case (which is great), and it is good that the test is
almost isolated.
It is absolutely not clear why do you add the test to an existing test
method. Is is complicated to add a new method?
Note: "long test methods" like `testSimplifyFilter` and
`testSimplifyOrTerms` were created only because it was complicated to
initialize `RexLiteral` and similar objects for the test before
`RexProgramBuilderBase` appeared .
Nowadays, you can use `literal(1)`, and you do not need `literal1` variable.
In the same way, you can use `vInt(0)` and `vInt(3)` which eliminates the
need for `ref0` and `ref3`.
Apparently `RexProgramBuilderBase` misses the DSL for
`SqlStdOperatorTable.ITEM` so you can leave it in your test method or add
`item` DSL to `RexProgramBuilderBase`.
Could you please consider moving the added test case to its own method and
use `vInt(...)` for variables?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
