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 05a7108)

Author: matriv

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

@@ -7,7 +7,6 @@
 
 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 @@ protected Pipe makePipe() {
 
     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(

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

@@ -7,7 +7,6 @@
 
 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 @@ protected NodeInfo<? extends Expression> info() {
         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);

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

@@ -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 @@ protected NodeInfo<? extends Expression> info() {
         return NodeInfo.create(this, DatePart::new, left(), right(), zoneId());
     }
 
-    @Override
-    public Nullability nullable() {
-        return Nullability.TRUE;
-    }
-
     @Override
     protected String scriptMethodName() {
         return "datePart";

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

@@ -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 @@ protected NodeInfo<? extends Expression> info() {
         return NodeInfo.create(this, DateTrunc::new, left(), right(), zoneId());
     }
 
-    @Override
-    public Nullability nullable() {
-        return Nullability.TRUE;
-    }
-
     @Override
     protected String scriptMethodName() {
         return "dateTrunc";

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

@@ -7,7 +7,6 @@
 
 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 @@ protected Pipe makePipe() {
 
     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();

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

@@ -35,6 +35,9 @@
 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,14 +617,39 @@ public void testSimplifyLeastRandomNullsWithValue() {
         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());
         assertEquals(1, foldNull.rule(new Concat(EMPTY, ONE, NULL)).fold());
         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 void testTranslateMaxToLast() {
         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,20 +1490,20 @@ public void testSortAggregateOnOrderByWithTwoFields() {
         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());
         assertTrue(order.get(0).child() instanceof FieldAttribute);
         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 void testSortAggregateOnOrderByWithTwoFields() {
         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,20 +1522,20 @@ public void testSortAggregateOnOrderByOnlyAliases() {
         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());
         assertTrue(order.get(0).child() instanceof Alias);
         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,28 +1564,28 @@ public void testPivotRewrite() {
         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')
      */
     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);
         List<FullTextPredicate> predicates = Arrays.asList(matchPredicate, multiMatchPredicate, stringQueryPredicate);
 
         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 void testAggregatesPromoteToStats_WithFullTextPredicatesConditions() {
         } 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 void testAggregatesPromoteToStats_WithFullTextPredicatesConditions() {
             assertEquals(((ExtendedStats) resultFirstAgg.outer()).field(), aggField);
             assertEquals(((ExtendedStats) resultSecondAgg.outer()).field(), aggField);
         }
-        
+
         assertTrue(resultAgg.child() instanceof Filter);
         assertEquals(resultAgg.child(), filter);
     }
-}
+}