Skip to content

Commit 56a4176

Browse files
committed
SQL: Fix issue with CAST and NULL checking. (#50371)
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 (cherry picked from commit 671e07a)
1 parent 72ca809 commit 56a4176

File tree

3 files changed

+50
-7
lines changed

3 files changed

+50
-7
lines changed

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/Cast.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ public Nullability nullable() {
6666
if (from().isNull()) {
6767
return Nullability.TRUE;
6868
}
69-
return field().nullable();
69+
return Nullability.UNKNOWN;
7070
}
7171

7272
@Override

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/predicate/nulls/CheckNullProcessorTests.java

-6
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,9 @@
99
import org.elasticsearch.common.io.stream.Writeable.Reader;
1010
import org.elasticsearch.test.AbstractWireSerializingTestCase;
1111
import org.elasticsearch.xpack.sql.expression.function.scalar.Processors;
12-
import org.elasticsearch.xpack.sql.expression.gen.processor.ConstantProcessor;
13-
import org.elasticsearch.xpack.sql.expression.gen.processor.Processor;
1412

1513
public class CheckNullProcessorTests extends AbstractWireSerializingTestCase<CheckNullProcessor> {
1614

17-
private static final Processor FALSE = new ConstantProcessor(false);
18-
private static final Processor TRUE = new ConstantProcessor(true);
19-
private static final Processor NULL = new ConstantProcessor((Object) null);
20-
2115
public static CheckNullProcessor randomProcessor() {
2216
return new CheckNullProcessor(randomFrom(CheckNullProcessor.CheckNullOperation.values()));
2317
}

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/optimizer/OptimizerTests.java

+49
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
package org.elasticsearch.xpack.sql.optimizer;
77

88
import org.elasticsearch.test.ESTestCase;
9+
import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
910
import org.elasticsearch.xpack.sql.analysis.analyzer.Analyzer.PruneSubqueryAliases;
1011
import org.elasticsearch.xpack.sql.analysis.index.EsIndex;
1112
import org.elasticsearch.xpack.sql.expression.Alias;
@@ -447,10 +448,46 @@ public void testNullFoldingIsNull() {
447448
assertEquals(false, foldNull.rule(new IsNull(EMPTY, TRUE)).fold());
448449
}
449450

451+
public void testNullFoldingIsNullWithCast() {
452+
FoldNull foldNull = new FoldNull();
453+
454+
Cast cast = new Cast(EMPTY, L("foo"), DataType.DATE);
455+
IsNull isNull = new IsNull(EMPTY, cast);
456+
final IsNull isNullOpt = (IsNull) foldNull.rule(isNull);
457+
assertEquals(isNull, isNullOpt);
458+
459+
SqlIllegalArgumentException sqlIAE =
460+
expectThrows(SqlIllegalArgumentException.class, () -> isNullOpt.asPipe().asProcessor().process(null));
461+
assertEquals("cannot cast [foo] to [date]: Text 'foo' could not be parsed at index 0", sqlIAE.getMessage());
462+
463+
isNull = new IsNull(EMPTY, new Cast(EMPTY, NULL, randomFrom(DataType.values())));
464+
assertTrue((Boolean) ((IsNull) foldNull.rule(isNull)).asPipe().asProcessor().process(null));
465+
}
466+
450467
public void testNullFoldingIsNotNull() {
451468
FoldNull foldNull = new FoldNull();
452469
assertEquals(true, foldNull.rule(new IsNotNull(EMPTY, TRUE)).fold());
453470
assertEquals(false, foldNull.rule(new IsNotNull(EMPTY, NULL)).fold());
471+
472+
Cast cast = new Cast(EMPTY, L("foo"), DataType.DATE);
473+
IsNotNull isNotNull = new IsNotNull(EMPTY, cast);
474+
assertEquals(isNotNull, foldNull.rule(isNotNull));
475+
}
476+
477+
public void testNullFoldingIsNotNullWithCast() {
478+
FoldNull foldNull = new FoldNull();
479+
480+
Cast cast = new Cast(EMPTY, L("foo"), DataType.DATE);
481+
IsNotNull isNotNull = new IsNotNull(EMPTY, cast);
482+
final IsNotNull isNotNullOpt = (IsNotNull) foldNull.rule(isNotNull);
483+
assertEquals(isNotNull, isNotNullOpt);
484+
485+
SqlIllegalArgumentException sqlIAE =
486+
expectThrows(SqlIllegalArgumentException.class, () -> isNotNullOpt.asPipe().asProcessor().process(null));
487+
assertEquals("cannot cast [foo] to [date]: Text 'foo' could not be parsed at index 0", sqlIAE.getMessage());
488+
489+
isNotNull = new IsNotNull(EMPTY, new Cast(EMPTY, NULL, randomFrom(DataType.values())));
490+
assertFalse((Boolean) ((IsNotNull) foldNull.rule(isNotNull)).asPipe().asProcessor().process(null));
454491
}
455492

456493
public void testGenericNullableExpression() {
@@ -470,6 +507,18 @@ public void testGenericNullableExpression() {
470507
assertNullLiteral(rule.rule(new RLike(EMPTY, NULL, "123")));
471508
}
472509

510+
public void testNullFoldingOnCast() {
511+
FoldNull foldNull = new FoldNull();
512+
513+
Cast cast = new Cast(EMPTY, NULL, randomFrom(DataType.values()));
514+
assertEquals(Nullability.TRUE, cast.nullable());
515+
assertNull(foldNull.rule(cast).fold());
516+
517+
cast = new Cast(EMPTY, L("foo"), DataType.DATE);
518+
assertEquals(Nullability.UNKNOWN, cast.nullable());
519+
assertEquals(cast, foldNull.rule(cast));
520+
}
521+
473522
public void testNullFoldingDoesNotApplyOnLogicalExpressions() {
474523
FoldNull rule = new FoldNull();
475524

0 commit comments

Comments
 (0)