SQL: Fix Nullability of DATEADD (#47921)
Previously, Nullability was set to UNKNOWN instead of TRUE which resulted on QueryFolder not correctly folding to NULL if any of the args was null. Remove the overriding nullable() also for DatePart/DateTrunc to allow delegation the parent class. (cherry picked from commit 05a7108e133b5ae7bec2257db5ae2d30ad926ee2)
This commit is contained in:
Marios Trivyzas
parent
ac209c142c
commit
65717f6f42
|
|
@ -7,7 +7,6 @@ package org.elasticsearch.xpack.sql.expression.function.scalar.datetime;
|
|||
|
||||
import org.elasticsearch.xpack.sql.expression.Expression;
|
||||
import org.elasticsearch.xpack.sql.expression.Expressions;
|
||||
import org.elasticsearch.xpack.sql.expression.Nullability;
|
||||
import org.elasticsearch.xpack.sql.expression.function.scalar.BinaryScalarFunction;
|
||||
import org.elasticsearch.xpack.sql.expression.gen.pipeline.Pipe;
|
||||
import org.elasticsearch.xpack.sql.expression.gen.script.ScriptTemplate;
|
||||
|
|
@ -79,11 +78,6 @@ public abstract class BinaryDateTimeFunction extends BinaryScalarFunction {
|
|||
|
||||
protected abstract Pipe createPipe(Pipe left, Pipe right, ZoneId zoneId);
|
||||
|
||||
@Override
|
||||
public Nullability nullable() {
|
||||
return Nullability.TRUE;
|
||||
}
|
||||
|
||||
@Override
|
||||
protected ScriptTemplate asScriptFrom(ScriptTemplate leftScript, ScriptTemplate rightScript) {
|
||||
return new ScriptTemplate(
|
||||
|
|
|
|||
|
|
@ -7,7 +7,6 @@ package org.elasticsearch.xpack.sql.expression.function.scalar.datetime;
|
|||
|
||||
import org.elasticsearch.xpack.sql.expression.Expression;
|
||||
import org.elasticsearch.xpack.sql.expression.Expressions;
|
||||
import org.elasticsearch.xpack.sql.expression.Nullability;
|
||||
import org.elasticsearch.xpack.sql.expression.gen.pipeline.Pipe;
|
||||
import org.elasticsearch.xpack.sql.tree.NodeInfo;
|
||||
import org.elasticsearch.xpack.sql.tree.Source;
|
||||
|
|
@ -136,11 +135,6 @@ public class DateAdd extends ThreeArgsDateTimeFunction {
|
|||
return NodeInfo.create(this, DateAdd::new, first(), second(), third(), zoneId());
|
||||
}
|
||||
|
||||
@Override
|
||||
public Nullability nullable() {
|
||||
return Nullability.UNKNOWN;
|
||||
}
|
||||
|
||||
@Override
|
||||
protected Pipe createPipe(Pipe first, Pipe second, Pipe third, ZoneId zoneId) {
|
||||
return new DateAddPipe(source(), this, first, second, third, zoneId);
|
||||
|
|
|
|||
|
|
@ -6,7 +6,6 @@
|
|||
package org.elasticsearch.xpack.sql.expression.function.scalar.datetime;
|
||||
|
||||
import org.elasticsearch.xpack.sql.expression.Expression;
|
||||
import org.elasticsearch.xpack.sql.expression.Nullability;
|
||||
import org.elasticsearch.xpack.sql.expression.function.scalar.BinaryScalarFunction;
|
||||
import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DateTimeProcessor.DateTimeExtractor;
|
||||
import org.elasticsearch.xpack.sql.expression.gen.pipeline.Pipe;
|
||||
|
|
@ -97,11 +96,6 @@ public class DatePart extends BinaryDateTimeFunction {
|
|||
return NodeInfo.create(this, DatePart::new, left(), right(), zoneId());
|
||||
}
|
||||
|
||||
@Override
|
||||
public Nullability nullable() {
|
||||
return Nullability.TRUE;
|
||||
}
|
||||
|
||||
@Override
|
||||
protected String scriptMethodName() {
|
||||
return "datePart";
|
||||
|
|
|
|||
|
|
@ -6,7 +6,6 @@
|
|||
package org.elasticsearch.xpack.sql.expression.function.scalar.datetime;
|
||||
|
||||
import org.elasticsearch.xpack.sql.expression.Expression;
|
||||
import org.elasticsearch.xpack.sql.expression.Nullability;
|
||||
import org.elasticsearch.xpack.sql.expression.function.scalar.BinaryScalarFunction;
|
||||
import org.elasticsearch.xpack.sql.expression.gen.pipeline.Pipe;
|
||||
import org.elasticsearch.xpack.sql.tree.NodeInfo;
|
||||
|
|
@ -153,11 +152,6 @@ public class DateTrunc extends BinaryDateTimeFunction {
|
|||
return NodeInfo.create(this, DateTrunc::new, left(), right(), zoneId());
|
||||
}
|
||||
|
||||
@Override
|
||||
public Nullability nullable() {
|
||||
return Nullability.TRUE;
|
||||
}
|
||||
|
||||
@Override
|
||||
protected String scriptMethodName() {
|
||||
return "dateTrunc";
|
||||
|
|
|
|||
|
|
@ -7,7 +7,6 @@ package org.elasticsearch.xpack.sql.expression.function.scalar.datetime;
|
|||
|
||||
import org.elasticsearch.xpack.sql.expression.Expression;
|
||||
import org.elasticsearch.xpack.sql.expression.Expressions;
|
||||
import org.elasticsearch.xpack.sql.expression.Nullability;
|
||||
import org.elasticsearch.xpack.sql.expression.function.scalar.ScalarFunction;
|
||||
import org.elasticsearch.xpack.sql.expression.gen.pipeline.Pipe;
|
||||
import org.elasticsearch.xpack.sql.expression.gen.script.ScriptTemplate;
|
||||
|
|
@ -69,11 +68,6 @@ public abstract class ThreeArgsDateTimeFunction extends ScalarFunction {
|
|||
|
||||
protected abstract Pipe createPipe(Pipe first, Pipe second, Pipe third, ZoneId zoneId);
|
||||
|
||||
@Override
|
||||
public Nullability nullable() {
|
||||
return Nullability.TRUE;
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean foldable() {
|
||||
return first().foldable() && second().foldable() && third().foldable();
|
||||
|
|
|
|||
|
|
@ -35,6 +35,9 @@ import org.elasticsearch.xpack.sql.expression.function.aggregate.Sum;
|
|||
import org.elasticsearch.xpack.sql.expression.function.aggregate.SumOfSquares;
|
||||
import org.elasticsearch.xpack.sql.expression.function.aggregate.VarPop;
|
||||
import org.elasticsearch.xpack.sql.expression.function.scalar.Cast;
|
||||
import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DateAdd;
|
||||
import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DatePart;
|
||||
import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DateTrunc;
|
||||
import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DayName;
|
||||
import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DayOfMonth;
|
||||
import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DayOfYear;
|
||||
|
|
@ -614,7 +617,7 @@ public class OptimizerTests extends ESTestCase {
|
|||
assertEquals(TWO, e.children().get(1));
|
||||
assertEquals(DataType.INTEGER, e.dataType());
|
||||
}
|
||||
|
||||
|
||||
public void testConcatFoldingIsNotNull() {
|
||||
FoldNull foldNull = new FoldNull();
|
||||
assertEquals(1, foldNull.rule(new Concat(EMPTY, NULL, ONE)).fold());
|
||||
|
|
@ -622,6 +625,31 @@ public class OptimizerTests extends ESTestCase {
|
|||
assertEquals(StringUtils.EMPTY, foldNull.rule(new Concat(EMPTY, NULL, NULL)).fold());
|
||||
}
|
||||
|
||||
public void testFoldNullDateAdd() {
|
||||
FoldNull foldNull = new FoldNull();
|
||||
assertNullLiteral(foldNull.rule(new DateAdd(EMPTY, NULL, TWO, THREE, UTC)));
|
||||
assertNullLiteral(foldNull.rule(new DateAdd(EMPTY, ONE, NULL, THREE, UTC)));
|
||||
assertNullLiteral(foldNull.rule(new DateAdd(EMPTY, ONE, TWO, NULL, UTC)));
|
||||
assertNullLiteral(foldNull.rule(new DateAdd(EMPTY, NULL, NULL, NULL, UTC)));
|
||||
assertTrue(foldNull.rule(new DateAdd(EMPTY, ONE, TWO, THREE, UTC)) instanceof DateAdd);
|
||||
}
|
||||
|
||||
public void testFoldNullDatePart() {
|
||||
FoldNull foldNull = new FoldNull();
|
||||
assertNullLiteral(foldNull.rule(new DatePart(EMPTY, NULL, TWO, UTC)));
|
||||
assertNullLiteral(foldNull.rule(new DatePart(EMPTY, ONE, NULL, UTC)));
|
||||
assertNullLiteral(foldNull.rule(new DatePart(EMPTY, NULL, NULL, UTC)));
|
||||
assertTrue(foldNull.rule(new DatePart(EMPTY, ONE, TWO, UTC)) instanceof DatePart);
|
||||
}
|
||||
|
||||
public void testFoldNullDateTrunc() {
|
||||
FoldNull foldNull = new FoldNull();
|
||||
assertNullLiteral(foldNull.rule(new DateTrunc(EMPTY, NULL, TWO, UTC)));
|
||||
assertNullLiteral(foldNull.rule(new DateTrunc(EMPTY, ONE, NULL, UTC)));
|
||||
assertNullLiteral(foldNull.rule(new DateTrunc(EMPTY, NULL, NULL, UTC)));
|
||||
assertTrue(foldNull.rule(new DateTrunc(EMPTY, ONE, TWO, UTC)) instanceof DateTrunc);
|
||||
}
|
||||
|
||||
public void testSimplifyCaseConditionsFoldWhenFalse() {
|
||||
// CASE WHEN a = 1 THEN 'foo1'
|
||||
// WHEN 1 = 2 THEN 'bar1'
|
||||
|
|
@ -1453,7 +1481,7 @@ public class OptimizerTests extends ESTestCase {
|
|||
assertSame(last, aggregates.get(0));
|
||||
assertEquals(max2, aggregates.get(1));
|
||||
}
|
||||
|
||||
|
||||
public void testSortAggregateOnOrderByWithTwoFields() {
|
||||
FieldAttribute firstField = new FieldAttribute(EMPTY, "first_field", new EsField("first_field", DataType.BYTE, emptyMap(), true));
|
||||
FieldAttribute secondField = new FieldAttribute(EMPTY, "second_field",
|
||||
|
|
@ -1462,12 +1490,12 @@ public class OptimizerTests extends ESTestCase {
|
|||
Alias secondAlias = new Alias(EMPTY, "second_alias", secondField);
|
||||
Order firstOrderBy = new Order(EMPTY, firstField, OrderDirection.ASC, Order.NullsPosition.LAST);
|
||||
Order secondOrderBy = new Order(EMPTY, secondField, OrderDirection.ASC, Order.NullsPosition.LAST);
|
||||
|
||||
|
||||
OrderBy orderByPlan = new OrderBy(EMPTY,
|
||||
new Aggregate(EMPTY, FROM(), Arrays.asList(secondField, firstField), Arrays.asList(secondAlias, firstAlias)),
|
||||
Arrays.asList(firstOrderBy, secondOrderBy));
|
||||
LogicalPlan result = new SortAggregateOnOrderBy().apply(orderByPlan);
|
||||
|
||||
|
||||
assertTrue(result instanceof OrderBy);
|
||||
List<Order> order = ((OrderBy) result).order();
|
||||
assertEquals(2, order.size());
|
||||
|
|
@ -1475,7 +1503,7 @@ public class OptimizerTests extends ESTestCase {
|
|||
assertTrue(order.get(1).child() instanceof FieldAttribute);
|
||||
assertEquals("first_field", ((FieldAttribute) order.get(0).child()).name());
|
||||
assertEquals("second_field", ((FieldAttribute) order.get(1).child()).name());
|
||||
|
||||
|
||||
assertTrue(((OrderBy) result).child() instanceof Aggregate);
|
||||
Aggregate agg = (Aggregate) ((OrderBy) result).child();
|
||||
List<?> groupings = agg.groupings();
|
||||
|
|
@ -1485,7 +1513,7 @@ public class OptimizerTests extends ESTestCase {
|
|||
assertEquals(firstField, groupings.get(0));
|
||||
assertEquals(secondField, groupings.get(1));
|
||||
}
|
||||
|
||||
|
||||
public void testSortAggregateOnOrderByOnlyAliases() {
|
||||
FieldAttribute firstField = new FieldAttribute(EMPTY, "first_field", new EsField("first_field", DataType.BYTE, emptyMap(), true));
|
||||
FieldAttribute secondField = new FieldAttribute(EMPTY, "second_field",
|
||||
|
|
@ -1494,12 +1522,12 @@ public class OptimizerTests extends ESTestCase {
|
|||
Alias secondAlias = new Alias(EMPTY, "second_alias", secondField);
|
||||
Order firstOrderBy = new Order(EMPTY, firstAlias, OrderDirection.ASC, Order.NullsPosition.LAST);
|
||||
Order secondOrderBy = new Order(EMPTY, secondAlias, OrderDirection.ASC, Order.NullsPosition.LAST);
|
||||
|
||||
|
||||
OrderBy orderByPlan = new OrderBy(EMPTY,
|
||||
new Aggregate(EMPTY, FROM(), Arrays.asList(secondAlias, firstAlias), Arrays.asList(secondAlias, firstAlias)),
|
||||
Arrays.asList(firstOrderBy, secondOrderBy));
|
||||
LogicalPlan result = new SortAggregateOnOrderBy().apply(orderByPlan);
|
||||
|
||||
|
||||
assertTrue(result instanceof OrderBy);
|
||||
List<Order> order = ((OrderBy) result).order();
|
||||
assertEquals(2, order.size());
|
||||
|
|
@ -1507,7 +1535,7 @@ public class OptimizerTests extends ESTestCase {
|
|||
assertTrue(order.get(1).child() instanceof Alias);
|
||||
assertEquals("first_alias", ((Alias) order.get(0).child()).name());
|
||||
assertEquals("second_alias", ((Alias) order.get(1).child()).name());
|
||||
|
||||
|
||||
assertTrue(((OrderBy) result).child() instanceof Aggregate);
|
||||
Aggregate agg = (Aggregate) ((OrderBy) result).child();
|
||||
List<?> groupings = agg.groupings();
|
||||
|
|
@ -1536,7 +1564,7 @@ public class OptimizerTests extends ESTestCase {
|
|||
assertEquals(column, in.value());
|
||||
assertEquals(Arrays.asList(L(1), L(2)), in.list());
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* Test queries like SELECT MIN(agg_field), MAX(agg_field) FROM table WHERE MATCH(match_field,'A') AND/OR QUERY('match_field:A')
|
||||
* or SELECT STDDEV_POP(agg_field), VAR_POP(agg_field) FROM table WHERE MATCH(match_field,'A') AND/OR QUERY('match_field:A')
|
||||
|
|
@ -1544,7 +1572,7 @@ public class OptimizerTests extends ESTestCase {
|
|||
public void testAggregatesPromoteToStats_WithFullTextPredicatesConditions() {
|
||||
FieldAttribute matchField = new FieldAttribute(EMPTY, "match_field", new EsField("match_field", DataType.TEXT, emptyMap(), true));
|
||||
FieldAttribute aggField = new FieldAttribute(EMPTY, "agg_field", new EsField("agg_field", DataType.INTEGER, emptyMap(), true));
|
||||
|
||||
|
||||
FullTextPredicate matchPredicate = new MatchQueryPredicate(EMPTY, matchField, "A", StringUtils.EMPTY);
|
||||
FullTextPredicate multiMatchPredicate = new MultiMatchQueryPredicate(EMPTY, "match_field", "A", StringUtils.EMPTY);
|
||||
FullTextPredicate stringQueryPredicate = new StringQueryPredicate(EMPTY, "match_field:A", StringUtils.EMPTY);
|
||||
|
|
@ -1552,12 +1580,12 @@ public class OptimizerTests extends ESTestCase {
|
|||
|
||||
FullTextPredicate left = randomFrom(predicates);
|
||||
FullTextPredicate right = randomFrom(predicates);
|
||||
|
||||
|
||||
BinaryLogic or = new Or(EMPTY, left, right);
|
||||
BinaryLogic and = new And(EMPTY, left, right);
|
||||
BinaryLogic condition = randomFrom(or, and);
|
||||
Filter filter = new Filter(EMPTY, FROM(), condition);
|
||||
|
||||
|
||||
List<AggregateFunction> aggregates;
|
||||
boolean isSimpleStats = randomBoolean();
|
||||
if (isSimpleStats) {
|
||||
|
|
@ -1576,13 +1604,13 @@ public class OptimizerTests extends ESTestCase {
|
|||
} else {
|
||||
result = new ReplaceAggsWithExtendedStats().apply(aggregatePlan);
|
||||
}
|
||||
|
||||
|
||||
assertTrue(result instanceof Aggregate);
|
||||
Aggregate resultAgg = (Aggregate) result;
|
||||
assertEquals(2, resultAgg.aggregates().size());
|
||||
assertTrue(resultAgg.aggregates().get(0) instanceof InnerAggregate);
|
||||
assertTrue(resultAgg.aggregates().get(1) instanceof InnerAggregate);
|
||||
|
||||
|
||||
InnerAggregate resultFirstAgg = (InnerAggregate) resultAgg.aggregates().get(0);
|
||||
InnerAggregate resultSecondAgg = (InnerAggregate) resultAgg.aggregates().get(1);
|
||||
assertEquals(resultFirstAgg.inner(), firstAggregate);
|
||||
|
|
@ -1598,8 +1626,8 @@ public class OptimizerTests extends ESTestCase {
|
|||
assertEquals(((ExtendedStats) resultFirstAgg.outer()).field(), aggField);
|
||||
assertEquals(((ExtendedStats) resultSecondAgg.outer()).field(), aggField);
|
||||
}
|
||||
|
||||
|
||||
assertTrue(resultAgg.child() instanceof Filter);
|
||||
assertEquals(resultAgg.child(), filter);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue