Skip to content

Commit

Permalink
SQL: Fix issue with CAST and NULL checking. (#50371)
Browse files Browse the repository at this point in the history
Previously, during expression optimisation, CAST would be considered
nullable if the casted expression resulted to a NULL literal, and would
be always non-nullable otherwise. As a result if CASE was wrapped by a
null check function like IS NULL or IS NOT NULL it was simplified to
TRUE/FALSE, eliminating the actual casting operation. So in case of an
expression with an erroneous casting like CAST('foo' AS DATETIME) IS NULL
it would be simplified to FALSE instead of throwing an Exception signifying
the attempt to cast 'foo' to a DATETIME type.

CAST now always returns Nullability.UKNOWN except from the case that
its result evaluated to a constant NULL, where it returns Nullability.TRUE.
This way the IS NULL/IS NOT NULL don't get simplified to FALSE/TRUE
and the CAST actually gets evaluated resulting to a thrown Exception.

Fixes: #50191
  • Loading branch information
matriv authored Dec 19, 2019
1 parent 1bffcf6 commit 671e07a
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public Nullability nullable() {
if (from().isNull()) {
return Nullability.TRUE;
}
return field().nullable();
return Nullability.UNKNOWN;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,9 @@
import org.elasticsearch.common.io.stream.Writeable.Reader;
import org.elasticsearch.test.AbstractWireSerializingTestCase;
import org.elasticsearch.xpack.sql.expression.function.scalar.Processors;
import org.elasticsearch.xpack.sql.expression.gen.processor.ConstantProcessor;
import org.elasticsearch.xpack.sql.expression.gen.processor.Processor;

public class CheckNullProcessorTests extends AbstractWireSerializingTestCase<CheckNullProcessor> {

private static final Processor FALSE = new ConstantProcessor(false);
private static final Processor TRUE = new ConstantProcessor(true);
private static final Processor NULL = new ConstantProcessor((Object) null);

public static CheckNullProcessor randomProcessor() {
return new CheckNullProcessor(randomFrom(CheckNullProcessor.CheckNullOperation.values()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package org.elasticsearch.xpack.sql.optimizer;

import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
import org.elasticsearch.xpack.sql.analysis.analyzer.Analyzer.PruneSubqueryAliases;
import org.elasticsearch.xpack.sql.analysis.index.EsIndex;
import org.elasticsearch.xpack.sql.expression.Alias;
Expand Down Expand Up @@ -438,10 +439,46 @@ public void testNullFoldingIsNull() {
assertEquals(false, foldNull.rule(new IsNull(EMPTY, TRUE)).fold());
}

public void testNullFoldingIsNullWithCast() {
FoldNull foldNull = new FoldNull();

Cast cast = new Cast(EMPTY, L("foo"), DataType.DATE);
IsNull isNull = new IsNull(EMPTY, cast);
final IsNull isNullOpt = (IsNull) foldNull.rule(isNull);
assertEquals(isNull, isNullOpt);

SqlIllegalArgumentException sqlIAE =
expectThrows(SqlIllegalArgumentException.class, () -> isNullOpt.asPipe().asProcessor().process(null));
assertEquals("cannot cast [foo] to [date]: Text 'foo' could not be parsed at index 0", sqlIAE.getMessage());

isNull = new IsNull(EMPTY, new Cast(EMPTY, NULL, randomFrom(DataType.values())));
assertTrue((Boolean) ((IsNull) foldNull.rule(isNull)).asPipe().asProcessor().process(null));
}

public void testNullFoldingIsNotNull() {
FoldNull foldNull = new FoldNull();
assertEquals(true, foldNull.rule(new IsNotNull(EMPTY, TRUE)).fold());
assertEquals(false, foldNull.rule(new IsNotNull(EMPTY, NULL)).fold());

Cast cast = new Cast(EMPTY, L("foo"), DataType.DATE);
IsNotNull isNotNull = new IsNotNull(EMPTY, cast);
assertEquals(isNotNull, foldNull.rule(isNotNull));
}

public void testNullFoldingIsNotNullWithCast() {
FoldNull foldNull = new FoldNull();

Cast cast = new Cast(EMPTY, L("foo"), DataType.DATE);
IsNotNull isNotNull = new IsNotNull(EMPTY, cast);
final IsNotNull isNotNullOpt = (IsNotNull) foldNull.rule(isNotNull);
assertEquals(isNotNull, isNotNullOpt);

SqlIllegalArgumentException sqlIAE =
expectThrows(SqlIllegalArgumentException.class, () -> isNotNullOpt.asPipe().asProcessor().process(null));
assertEquals("cannot cast [foo] to [date]: Text 'foo' could not be parsed at index 0", sqlIAE.getMessage());

isNotNull = new IsNotNull(EMPTY, new Cast(EMPTY, NULL, randomFrom(DataType.values())));
assertFalse((Boolean) ((IsNotNull) foldNull.rule(isNotNull)).asPipe().asProcessor().process(null));
}

public void testGenericNullableExpression() {
Expand All @@ -461,6 +498,18 @@ public void testGenericNullableExpression() {
assertNullLiteral(rule.rule(new RLike(EMPTY, NULL, "123")));
}

public void testNullFoldingOnCast() {
FoldNull foldNull = new FoldNull();

Cast cast = new Cast(EMPTY, NULL, randomFrom(DataType.values()));
assertEquals(Nullability.TRUE, cast.nullable());
assertNull(foldNull.rule(cast).fold());

cast = new Cast(EMPTY, L("foo"), DataType.DATE);
assertEquals(Nullability.UNKNOWN, cast.nullable());
assertEquals(cast, foldNull.rule(cast));
}

public void testNullFoldingDoesNotApplyOnLogicalExpressions() {
FoldNull rule = new FoldNull();

Expand Down Expand Up @@ -1684,4 +1733,4 @@ public void testReplaceAttributesWithTarget() {
gt = (GreaterThan) and.left();
assertEquals(a, gt.left());
}
}
}

0 comments on commit 671e07a

Please sign in to comment.