kgyrtkirk commented on a change in pull request #1322: [CALCITE-3198]
ReduceExpressionsRule.FILTER_INSTANCE does not reduce 'NOT(x=a AND x=b)'
URL: https://github.com/apache/calcite/pull/1322#discussion_r307614482
##########
File path: core/src/test/java/org/apache/calcite/test/RexProgramTest.java
##########
@@ -1493,6 +1493,83 @@ private void checkExponentialCnf(int n) {
"false");
}
+ /** Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-3198";>[CALCITE-3198]
+ * ReduceExpressionsRule.FILTER_INSTANCE does not reduce 'NOT(x=a AND
x=b)'</a>. */
+ @Test public void testSimplify3198() {
+ final RelDataType intType = typeFactory.createSqlType(SqlTypeName.INTEGER);
+ final RelDataType rowType = typeFactory.builder()
+ .add("a", intType).nullable(false)
+ .add("b", intType).nullable(true)
+ .build();
+
+ final RexDynamicParam range = rexBuilder.makeDynamicParam(rowType, 0);
Review comment:
* please use the other api which doesnt require all these things when
writing tests `vInt()` , `literal(1)` ; for a testcase like that see for
example `testSimplifyCaseCompaction2`
* `checkSimplifyFilter` is a specialized one; please use `checkSimplify`
instead - this should work in all unknownas modes properly
* please name the testcase about what its trying to do... "3198" doesn't
describe whats being tested..
* please don't add much redundant testcases: checking `not(and( exprs )`
when you have `or( exprs )` doesn't really a new test
* having all test cases a separate method has the benefit of debugging them
is easier when they fail
----------------------------------------------------------------
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
