Skip to content

[CALCITE-2438] RexCall#isAlwaysTrue return incorrect result #778

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jiayuanv127 wants to merge 10 commits into from
Closed

[CALCITE-2438] RexCall#isAlwaysTrue return incorrect result #778

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8c5ea5a
add table function test case
Jun 27, 2018
c9ee30e
retract
Jul 16, 2018
cedc254
Merge pull request #1 from apache/master
jiayuanv127 Jul 16, 2018
70def02
[CALCITE-2424] AggregateProjectPullUpConstantsRule throws AssertionE…
Jul 23, 2018
1c5e7d8
Merge pull request #2 from apache/master
jiayuanv127 Aug 2, 2018
1cd2403
[CALCITE-2438] RexCall#isAlwaysTrue return incorrect result
Aug 4, 2018
b7cc848
clear empty line
Aug 4, 2018
d1d6053
remove white space
Aug 6, 2018
c2a0cd7
add more test case
Aug 7, 2018
966227d
modify error message
Aug 7, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions core/src/main/java/org/apache/calcite/rex/RexCall.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,11 +121,12 @@ public RelDataType getType() {
switch (getKind()) {
case IS_NOT_NULL:
return !operands.get(0).getType().isNullable();
case IS_NOT_FALSE:
case NOT:
return operands.get(0).isAlwaysFalse();
case IS_NOT_TRUE:
case IS_FALSE:
case NOT:
return operands.get(0).isAlwaysFalse();
case IS_NOT_FALSE:
case IS_TRUE:
case CAST:
return operands.get(0).isAlwaysTrue();
default:
Expand All @@ -138,6 +139,7 @@ public RelDataType getType() {
case IS_NULL:
return !operands.get(0).getType().isNullable();
case IS_NOT_TRUE:
case IS_FALSE:
case NOT:
return operands.get(0).isAlwaysTrue();
case IS_NOT_FALSE:
Expand Down
65 changes: 62 additions & 3 deletions core/src/test/java/org/apache/calcite/test/RexProgramTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,22 @@ private RexNode isNotNull(RexNode node) {
return rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_NULL, node);
}

private RexNode isFalse(RexNode node) {
return rexBuilder.makeCall(SqlStdOperatorTable.IS_FALSE, node);
}

private RexNode isTrue(RexNode node) {
return rexBuilder.makeCall(SqlStdOperatorTable.IS_TRUE, node);
}

private RexNode isNotTrue(RexNode node) {
Copy link
Contributor

@vlsi vlsi Aug 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please co-locate is*True/False methods? It looks like isNotFalse is declared elsewhere

Copy link
Author

@jiayuanv127 jiayuanv127 Aug 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I have added the isNotFalse method after isNotTrue.

return rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_TRUE, node);
}

private RexNode isNotFalse(RexNode node) {
return rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_FALSE, node);
}

private RexNode nullIf(RexNode node1, RexNode node2) {
return rexBuilder.makeCall(SqlStdOperatorTable.NULLIF, node1, node2);
}
Expand Down Expand Up @@ -568,7 +584,7 @@ private RexProgramBuilder createProg(int variant) {
// $t15 = $14 is true
final RexLocalRef t15 =
builder.addExpr(
rexBuilder.makeCall(SqlStdOperatorTable.IS_TRUE, t14));
isTrue(t14));
builder.addCondition(t15);
}
}
Expand Down Expand Up @@ -2128,7 +2144,7 @@ private static String getString(ImmutableMap<RexNode, RexNode> map) {
typeFactory.createTypeWithNullability(
typeFactory.createSqlType(SqlTypeName.BOOLEAN), true);
final RexNode booleanInput = rexBuilder.makeInputRef(booleanNullableType, 0);
final RexNode isFalse = rexBuilder.makeCall(SqlStdOperatorTable.IS_FALSE, booleanInput);
final RexNode isFalse = isFalse(booleanInput);
final RexCall result = (RexCall) simplify(isFalse);
assertThat(result.getType().isNullable(), is(false));
assertThat(result.getOperator(), is((SqlOperator) SqlStdOperatorTable.IS_FALSE));
Expand All @@ -2138,7 +2154,7 @@ private static String getString(ImmutableMap<RexNode, RexNode> map) {
// Make sure that IS_FALSE(IS_FALSE(nullable boolean)) != IS_TRUE(nullable boolean)
// IS_FALSE(IS_FALSE(null)) = IS_FALSE(false) = true
// IS_TRUE(null) = false
final RexNode isFalseIsFalse = rexBuilder.makeCall(SqlStdOperatorTable.IS_FALSE, isFalse);
final RexNode isFalseIsFalse = isFalse(isFalse);
final RexCall result2 = (RexCall) simplify(isFalseIsFalse);
assertThat(result2.getType().isNullable(), is(false));
assertThat(result2.getOperator(), is((SqlOperator) SqlStdOperatorTable.IS_NOT_FALSE));
Expand Down Expand Up @@ -2170,6 +2186,49 @@ private RexNode simplify(RexNode e) {
is(false));
}

@Test public void testIsAlwaysTrueAndFalse() {
final RelDataType type =
typeFactory.createSqlType(SqlTypeName.BOOLEAN);
RexNode inputRef = rexBuilder.makeInputRef(type, 0);

RexNode isFalseExp = isFalse(isNotNull(isNull(inputRef)));
assertEquals("expected false,but got true in isFalseExp.isAlwaysTrue()",
isFalseExp.isAlwaysTrue(), false);
assertEquals("expected true,but got false in isFalseExp.isAlwaysFalse()",
isFalseExp.isAlwaysFalse(), true);

RexNode isFalseExp2 = isFalse(not(isNotNull(isNull(inputRef))));
assertEquals("expected true,but got false in isFalseExp2.isAlwaysTrue()",
isFalseExp2.isAlwaysTrue(), true);
assertEquals("expected false,but got true in isFalseExp2.isAlwaysFalse()",
isFalseExp2.isAlwaysFalse(), false);

RexNode isTrueExp = isTrue(isNotNull(isNull(inputRef)));
assertEquals("expected true,but got false in isTrueExp.isAlwaysTrue()",
isTrueExp.isAlwaysTrue(), true);
assertEquals("expected false,but got true in isTrueExp.isAlwaysFalse()",
isTrueExp.isAlwaysFalse(), false);

RexNode isTrueExp2 = isTrue(not(isNotNull(isNull(inputRef))));
assertEquals("expected false,but got true in isTrueExp2.isAlwaysTrue()",
isTrueExp2.isAlwaysTrue(), false);
assertEquals("expected true,but got false in isTrueExp2.isAlwaysFalse()",
isTrueExp2.isAlwaysFalse(), true);

RexNode isNotTrueExp = isNotTrue(isNotNull(isNull(inputRef)));
assertEquals("expected false,but got true in isNotTrueExp.isAlwaysTrue()",
isNotTrueExp.isAlwaysTrue(), false);
assertEquals("expected true,but got false in isNotTrueExp.isAlwaysFalse()",
isNotTrueExp.isAlwaysFalse(), true);

RexNode isNotTrueExp2 = isNotTrue(not(isNotNull(isNull(inputRef))));
assertEquals("expected true,but got false in isNotTrueExp2.isAlwaysTrue()",
isNotTrueExp2.isAlwaysTrue(), true);
assertEquals("expected false,but got true in isNotTrueExp2.isAlwaysFalse()",
isNotTrueExp2.isAlwaysFalse(), false);

}

private Comparable eval(RexNode e) {
return RexInterpreter.evaluate(e, ImmutableMap.of());
}
Expand Down