ESQL: Remove the swapped-args check for date_xxx() (elastic#101362)

This removes the check and helper error message for some of the `date_xxx()` functions whose args have been swapped for consistency. Close elastic#99562
carlosdelest · Oct 27, 2023 · debe882 · debe882
1 parent 6571d39
commit debe882
Show file tree

Hide file tree

Showing 6 changed files with 11 additions and 76 deletions.
diff --git a/docs/changelog/101362.yaml b/docs/changelog/101362.yaml
@@ -0,0 +1,6 @@
+pr: 101362
+summary: "ESQL: Remove the swapped-args check for date_xxx()"
+area: ES|QL
+type: enhancement
+issues:
+ - 99562
diff --git a/.../org/elasticsearch/xpack/esql/expression/function/scalar/date/BinaryDateTimeFunction.java b/.../org/elasticsearch/xpack/esql/expression/function/scalar/date/BinaryDateTimeFunction.java
@@ -17,9 +17,6 @@
 import java.time.ZoneId;
 import java.time.ZoneOffset;
 import java.util.Objects;
-import java.util.function.Predicate;
-
-import static org.elasticsearch.common.logging.LoggerMessageFormat.format;
 
 public abstract class BinaryDateTimeFunction extends BinaryScalarFunction {
 
@@ -69,12 +66,4 @@ public boolean equals(Object o) {
         BinaryDateTimeFunction that = (BinaryDateTimeFunction) o;
         return zoneId().equals(that.zoneId());
     }
-
-    // TODO: drop check once 8.11 is released
-    static TypeResolution argumentTypesAreSwapped(DataType left, DataType right, Predicate<DataType> rightTest, String source) {
-        if (DataTypes.isDateTime(left) && rightTest.test(right)) {
-            return new TypeResolution(format(null, "function definition has been updated, please swap arguments in [{}]", source));
-        }
-        return TypeResolution.TYPE_RESOLVED;
-    }
 }
diff --git a/...c/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/date/DateExtract.java b/...c/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/date/DateExtract.java
@@ -31,7 +31,6 @@
 import java.util.Locale;
 import java.util.function.Function;
 
-import static org.elasticsearch.xpack.esql.expression.function.scalar.date.BinaryDateTimeFunction.argumentTypesAreSwapped;
 import static org.elasticsearch.xpack.ql.expression.TypeResolutions.isDate;
 import static org.elasticsearch.xpack.ql.expression.TypeResolutions.isStringAndExact;
 
@@ -109,25 +108,9 @@ protected TypeResolution resolveType() {
         if (childrenResolved() == false) {
             return new TypeResolution("Unresolved children");
         }
-        TypeResolution resolution = argumentTypesAreSwapped(
-            children().get(0).dataType(),
-            children().get(1).dataType(),
-            DataTypes::isString,
-            sourceText()
+        return isStringAndExact(children().get(0), sourceText(), TypeResolutions.ParamOrdinal.FIRST).and(
+            isDate(children().get(1), sourceText(), TypeResolutions.ParamOrdinal.SECOND)
         );
-        if (resolution.unresolved()) {
-            return resolution;
-        }
-        resolution = isStringAndExact(children().get(0), sourceText(), TypeResolutions.ParamOrdinal.FIRST);
-        if (resolution.unresolved()) {
-            return resolution;
-        }
-        resolution = isDate(children().get(1), sourceText(), TypeResolutions.ParamOrdinal.SECOND);
-        if (resolution.unresolved()) {
-            return resolution;
-        }
-
-        return TypeResolution.TYPE_RESOLVED;
     }
 
     @Override

diff --git a/...rc/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/date/DateFormat.java b/...rc/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/date/DateFormat.java
@@ -28,7 +28,6 @@
 import java.util.Locale;
 import java.util.function.Function;
 
-import static org.elasticsearch.xpack.esql.expression.function.scalar.date.BinaryDateTimeFunction.argumentTypesAreSwapped;
 import static org.elasticsearch.xpack.ql.expression.TypeResolutions.ParamOrdinal.FIRST;
 import static org.elasticsearch.xpack.ql.expression.TypeResolutions.ParamOrdinal.SECOND;
 import static org.elasticsearch.xpack.ql.expression.TypeResolutions.isDate;
@@ -57,15 +56,7 @@ protected TypeResolution resolveType() {
             return new TypeResolution("Unresolved children");
         }
 
-        TypeResolution resolution;
-        if (format != null) {
-            resolution = argumentTypesAreSwapped(format.dataType(), field.dataType(), DataTypes::isString, sourceText());
-            if (resolution.unresolved()) {
-                return resolution;
-            }
-        }
-
-        resolution = isDate(field, sourceText(), format == null ? FIRST : SECOND);
+        TypeResolution resolution = isDate(field, sourceText(), format == null ? FIRST : SECOND);
         if (resolution.unresolved()) {
             return resolution;
         }

diff --git a/...src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/date/DateTrunc.java b/...src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/date/DateTrunc.java
@@ -42,22 +42,9 @@ protected TypeResolution resolveType() {
             return new TypeResolution("Unresolved children");
         }
 
-        TypeResolution resolution = argumentTypesAreSwapped(
-            left().dataType(),
-            right().dataType(),
-            EsqlDataTypes::isTemporalAmount,
-            sourceText()
+        return isDate(timestampField(), sourceText(), FIRST).and(
+            isType(interval(), EsqlDataTypes::isTemporalAmount, sourceText(), SECOND, "dateperiod", "timeduration")
         );
-        if (resolution.unresolved()) {
-            return resolution;
-        }
-
-        resolution = isDate(timestampField(), sourceText(), FIRST);
-        if (resolution.unresolved()) {
-            return resolution;
-        }
-
-        return isType(interval(), EsqlDataTypes::isTemporalAmount, sourceText(), SECOND, "dateperiod", "timeduration");
     }
 
     @Override

diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java
@@ -1031,27 +1031,6 @@ public void testDateTruncWithNumericInterval() {
             """, "second argument of [date_trunc(1, date)] must be [dateperiod or timeduration], found value [1] type [integer]");
     }
 
-    public void testDateExtractWithSwappedArguments() {
-        verifyUnsupported("""
-            from test
-            | eval date_extract(date, "year")
-            """, "function definition has been updated, please swap arguments in [date_extract(date, \"year\")]");
-    }
-
-    public void testDateFormatWithSwappedArguments() {
-        verifyUnsupported("""
-            from test
-            | eval date_format(date, "yyyy-MM-dd")
-            """, "function definition has been updated, please swap arguments in [date_format(date, \"yyyy-MM-dd\")]");
-    }
-
-    public void testDateTruncWithSwappedArguments() {
-        verifyUnsupported("""
-            from test
-            | eval date_trunc(date, 1 month)
-            """, "function definition has been updated, please swap arguments in [date_trunc(date, 1 month)]");
-    }
-
     public void testDateTruncWithDateInterval() {
         verifyUnsupported("""
             from test