[
https://issues.apache.org/jira/browse/CALCITE-3457?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16964226#comment-16964226
]
Vova Vysotskyi commented on CALCITE-3457:
-----------------------------------------
I tried to add this assertion, but it will cause false-positive failures. Here
are the methods I have added (if I understand the intention correctly):
{code:java}
/**
* Validates strong policy for specified {@link RexNode} rex node.
*
* @param rexNode rex node whose strong policy should be validated.
*/
private void validateStrongPolicy(RexNode rexNode) {
if (hasCustomNullabilityRules(rexNode.getKind())) {
return;
}
switch (Strong.policy(rexNode.getKind())) {
case NOT_NULL:
assert !rexNode.getType().isNullable();
break;
case ANY:
List<RexNode> operands = ((RexCall) rexNode).getOperands();
if (rexNode.getType().isNullable()) {
assert operands.stream()
.map(RexNode::getType)
.anyMatch(RelDataType::isNullable);
} else {
assert operands.stream()
.map(RexNode::getType)
.noneMatch(RelDataType::isNullable);
}
}
}
/**
* Returns {@code true} if specified {@link SqlKind} sql kind has custom
nullability rules.
*
* @param sqlKind sql kind to check
* @return {@code true} if specified {@link SqlKind} sql kind has custom
nullability rules
*/
private boolean hasCustomNullabilityRules(SqlKind sqlKind) {
switch (sqlKind) {
case CAST:
case ITEM:
return true;
default:
return false;
}
}
{code}
These assertions caused failures for the following case (simplified case from
{{RexProgramFuzzyTest}} failure):
{code:java}
@Test public void testSimplifyCastUnaryMinus() {
RexNode expr =
isNull(ne(unaryMinus(cast(unaryMinus(vIntNotNull(1)),
nullable(tInt()))), vIntNotNull(1)));
RexNode s = simplify.simplifyUnknownAs(expr, RexUnknownAs.UNKNOWN);
assertThat(s, is(falseLiteral));
}
{code}
It will fail for a call with {{MINUS}} function since the cast was simplified
to rex node with another type (looks like it was done in CALCITE-2695) and we
will end up with {{MINUS}} which still has a nullable type and cast argument
with non-nullable type.
I don't think that we should add this check since it actually conflicts with
the fix done in CALCITE-2695.
> RexSimplify incorrectly simplifies IS NOT NULL operator with ITEM call
> ----------------------------------------------------------------------
>
> Key: CALCITE-3457
> URL: https://issues.apache.org/jira/browse/CALCITE-3457
> Project: Calcite
> Issue Type: Bug
> Affects Versions: 1.22.0
> Reporter: Vova Vysotskyi
> Priority: Major
> Labels: pull-request-available
> Fix For: 1.22.0
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> In CALCITE-3390 ITEM was marked with {{Policy.ANY}} strong policy, but
> according to its JavaDoc, the result may be null if and only if at least one
> of its arguments is null. This statement was used in
> {{RexSimplify.simplifyIsNotNull()}} method, so {{t1.c_nationkey[0] is not
> null}} will be simplified to {{IS NOT NULL($0)}} which is wrong, since array
> may be empty, or index may be less than the size of the array.
> Unit test which helps to reproduce this issue:
> {noformat}
> @Test public void testSimplifyItemIsNotNull() {
> String query = "select * from sales.customer as t1 where
> t1.c_nationkey[0] is not null";
> sql(query)
> .withTester(t -> createDynamicTester())
> .withRule(ReduceExpressionsRule.FILTER_INSTANCE)
> .check();
> }
> {noformat}
> Returns plan with incorrectly simplified ITEM expression:
> {noformat}
> LogicalProject(**=[$1])
> LogicalFilter(condition=[IS NOT NULL($0)])
> LogicalTableScan(table=[[CATALOG, SALES, CUSTOMER]])
> {noformat}
> But the initial intention of CALCITE-3390 was to allow pushing ITEM
> expression to the right input of left-outer-join.
> I propose to add a new element to the {{Policy}} which will have a relaxed
> condition - expression is null if at least one of its arguments is null.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
