SQL: Make extract work for any datetime function (elastic/x-pack-elasticsearch#3756)

This allows any datetime function to be present in `EXTRACT` which feels
more consistent. `EXTRACT(FOO FROM bar)` is now just sugar for
`FOO(bar)`. This is *much* simpler to explain in the documentation then
"these 10 fields are supported by extract and they are the same as this
subset of the datetime functions."

The implementation of this is a little simpler then the old way. Instead
of resolving the function in the parser we create an
`UnresolvedFunction` that looks *almost* just like what we'd create for
a single argument function and resolve the function in the `Analyzer`.
This feels like a net positive as it allows us to group `EXTRACT`
resolution failures with other function resolution failures.

This also creates `UnresolvedFunctionTests` and
`UnresolvedAttributeTests`. I had to create `UnresolvedFunctionTests`
because `UnreolvedFunction` now has three boolean parameters which is
incompatible with the generic `NodeSubclassTests`'s requirement that all
ctor parameters be unique. I created `UnresolvedAttributeTests` because
I didn't want `UnresolvedFunctionTests` to call `NodeSubclassTests` and
figured that we'd want `UnresolvedAttributeTest` eventually and now felt
like as good a time as any.

Added a 

Original commit: elastic/x-pack-elasticsearch@358aada308

docs/en/sql/language/syntax.asciidoc

@@ -12,7 +12,7 @@ include-tagged::{sql-specs}/select.sql-spec[wildcardWithOrder]
 
 
 [[sql-spec-syntax-order-by]]
-==== ORDER BY
+==== `ORDER BY`
 
 Elasticsearch supports `ORDER BY` for consistent ordering. You add
 any field in the index that has <<doc-values,`doc_values`>> or
@@ -101,3 +101,23 @@ POST /_xpack/sql
 --------------------------------------------------
 // TESTRESPONSE[s/\|/\\|/ s/\+/\\+/ s/\(/\\\(/ s/\)/\\\)/]
 // TESTRESPONSE[_cat]
+
+
+[[sql-spec-syntax-extract]]
+==== `EXTRACT`
+
+Elasticsearch supports `EXTRACT` to extract fields from datetimes.
+You can run any <<sql-functions-datetime,datetime function>>
+with `EXTRACT(<datetime_function> FROM <expression>)`. So
+
+["source","sql",subs="attributes,callouts,macros"]
+--------------------------------------------------
+include-tagged::{sql-specs}/datetime.csv-spec[extractDayOfYear]
+--------------------------------------------------
+
+is the equivalent to
+
+["source","sql",subs="attributes,callouts,macros"]
+--------------------------------------------------
+include-tagged::{sql-specs}/datetime.csv-spec[dayOfYear]
+--------------------------------------------------

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Analyzer.java

@@ -763,14 +763,9 @@ public class Analyzer extends RuleExecutor<LogicalPlan> {
                     String name = uf.name();
 
                     if (hasStar(uf.arguments())) {
-                        if (uf.distinct()) {
+                        uf = uf.preprocessStar();
-                            throw new AnalysisException(uf, "DISTINCT and wildcard/star are not compatible");
+                        if (uf.analyzed()) {
-                        }
+                            return uf;
-                        // TODO: might be removed
-                        // dedicated count optimization
-                        if (name.toUpperCase(Locale.ROOT).equals("COUNT")) {
-                            uf = new UnresolvedFunction(uf.location(), uf.name(), uf.distinct(),
-                                singletonList(Literal.of(uf.arguments().get(0).location(), Integer.valueOf(1))));
                         }
                     }
 
@@ -792,21 +787,11 @@ public class Analyzer extends RuleExecutor<LogicalPlan> {
 
                     // not seen before, use the registry
                     if (!functionRegistry.functionExists(name)) {
-
+                        return uf.missing(normalizedName, functionRegistry.listFunctions());
-                        // try to find alternatives
-                        Set<String> names = new LinkedHashSet<>();
-                        for (FunctionDefinition def : functionRegistry.listFunctions()) {
-                            names.add(def.name());
-                            names.addAll(def.aliases());
-                        }
-
-                        List<String> matches = StringUtils.findSimilar(normalizedName, names);
-                        String message = matches.isEmpty() ?
-                                uf.unresolvedMessage() : UnresolvedFunction.errorMessage(normalizedName, matches);
-                        return new UnresolvedFunction(uf.location(), uf.name(), uf.distinct(), uf.children(), true, message);
                     }
                     // TODO: look into Generator for significant terms, etc..
-                    Function f = functionRegistry.resolveFunction(uf, timeZone);
+                    FunctionDefinition def = functionRegistry.resolveFunction(normalizedName);
+                    Function f = uf.buildResolved(timeZone, def);
 
                     list.add(f);
                     return f;

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/FunctionDefinition.java

@@ -7,25 +7,34 @@ package org.elasticsearch.xpack.sql.expression.function;
 
 import java.util.List;
 import java.util.Locale;
-import java.util.Objects;
-import java.util.function.BiFunction;
 import org.joda.time.DateTimeZone;
 
 import static java.lang.String.format;
 
 public class FunctionDefinition {
-
+    /**
+     * Converts an {@link UnresolvedFunction} into the a proper {@link Function}.
+     */
+    @FunctionalInterface
+    public interface Builder {
+        Function build(UnresolvedFunction uf, boolean distinct, DateTimeZone tz);
+    }
     private final String name;
     private final List<String> aliases;
     private final Class<? extends Function> clazz;
-    private final BiFunction<UnresolvedFunction, DateTimeZone, Function> builder;
+    /**
+     * Is this a datetime function comaptible with {@code EXTRACT}.
+     */
+    private final boolean datetime;
+    private final Builder builder;
     private final FunctionType type;
 
-    FunctionDefinition(String name, List<String> aliases,
+    FunctionDefinition(String name, List<String> aliases, Class<? extends Function> clazz,
-            Class<? extends Function> clazz, BiFunction<UnresolvedFunction, DateTimeZone, Function> builder) {
+            boolean datetime, Builder builder) {
         this.name = name;
         this.aliases = aliases;
         this.clazz = clazz;
+        this.datetime = datetime;
         this.builder = builder;
         this.type = FunctionType.of(clazz);
     }
@@ -46,10 +55,17 @@ public class FunctionDefinition {
         return clazz;
     }
 
-    BiFunction<UnresolvedFunction, DateTimeZone, Function> builder() {
+    Builder builder() {
         return builder;
     }
 
+    /**
+     * Is this a datetime function comaptible with {@code EXTRACT}.
+     */
+    boolean datetime() {
+        return datetime;
+    }
+
     @Override
     public String toString() {
         return format(Locale.ROOT, "%s(%s)", name, aliases.isEmpty() ? "" : aliases.size() == 1 ? aliases.get(0) : aliases );

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/FunctionRegistry.java

@@ -164,12 +164,12 @@ public class FunctionRegistry {
         }
     }
 
-    public Function resolveFunction(UnresolvedFunction ur, DateTimeZone timeZone) {
+    public FunctionDefinition resolveFunction(String name) {
-        FunctionDefinition def = defs.get(normalize(ur.name()));
+        FunctionDefinition def = defs.get(normalize(name));
         if (def == null) {
-            throw new SqlIllegalArgumentException("Cannot find function {}; this should have been caught during analysis", ur.name());
+            throw new SqlIllegalArgumentException("Cannot find function {}; this should have been caught during analysis", name);
         }
-        return def.builder().apply(ur, timeZone);
+        return def;
     }
 
     public String concreteFunctionName(String alias) {
@@ -182,16 +182,20 @@ public class FunctionRegistry {
     }
 
     public Collection<FunctionDefinition> listFunctions() {
+        // It is worth double checking if we need this copy. These are immutable anyway.
         return defs.entrySet().stream()
-                .map(e -> new FunctionDefinition(e.getKey(), emptyList(), e.getValue().clazz(), e.getValue().builder()))
+                .map(e -> new FunctionDefinition(e.getKey(), emptyList(),
+                        e.getValue().clazz(), e.getValue().datetime(), e.getValue().builder()))
                 .collect(toList());
     }
 
     public Collection<FunctionDefinition> listFunctions(String pattern) {
+        // It is worth double checking if we need this copy. These are immutable anyway.
         Pattern p = Strings.hasText(pattern) ? Pattern.compile(normalize(pattern)) : null;
         return defs.entrySet().stream()
                 .filter(e -> p == null || p.matcher(e.getKey()).matches())
-                .map(e -> new FunctionDefinition(e.getKey(), emptyList(), e.getValue().clazz(), e.getValue().builder()))
+                .map(e -> new FunctionDefinition(e.getKey(), emptyList(),
+                        e.getValue().clazz(), e.getValue().datetime(), e.getValue().builder()))
                 .collect(toList());
     }
 
@@ -210,7 +214,7 @@ public class FunctionRegistry {
             }
             return ctorRef.apply(location);
         };
-        return def(function, builder, aliases);
+        return def(function, builder, false, aliases);
     }
 
     /**
@@ -229,7 +233,7 @@ public class FunctionRegistry {
             }
             return ctorRef.apply(location, children.get(0));
         };
-        return def(function, builder, aliases);
+        return def(function, builder, false, aliases);
     }
 
     /**
@@ -245,19 +249,19 @@ public class FunctionRegistry {
             }
             return ctorRef.build(location, children.get(0), distinct);
         };
-        return def(function, builder, aliases);
+        return def(function, builder, false, aliases);
     }
     interface DistinctAwareUnaryFunctionBuilder<T> {
         T build(Location location, Expression target, boolean distinct);
     }
 
     /**
-     * Build a {@linkplain FunctionDefinition} for a unary function that is
+     * Build a {@linkplain FunctionDefinition} for a unary function that
-     * aware of time zone and does not support {@code DISTINCT}.
+     * operates on a datetime.
      */
     @SuppressWarnings("overloads")  // These are ambiguous if you aren't using ctor references but we always do
     static <T extends Function> FunctionDefinition def(Class<T> function,
-            TimeZoneAwareUnaryFunctionBuilder<T> ctorRef, String... aliases) {
+            DatetimeUnaryFunctionBuilder<T> ctorRef, String... aliases) {
         FunctionBuilder builder = (location, children, distinct, tz) -> {
             if (children.size() != 1) {
                 throw new IllegalArgumentException("expects exactly one argument");
@@ -267,9 +271,9 @@ public class FunctionRegistry {
             }
             return ctorRef.build(location, children.get(0), tz);
         };
-        return def(function, builder, aliases);
+        return def(function, builder, true, aliases);
     }
-    interface TimeZoneAwareUnaryFunctionBuilder<T> {
+    interface DatetimeUnaryFunctionBuilder<T> {
         T build(Location location, Expression target, DateTimeZone tz);
     }
 
@@ -289,23 +293,24 @@ public class FunctionRegistry {
             }
             return ctorRef.build(location, children.get(0), children.get(1));
         };
-        return def(function, builder, aliases);
+        return def(function, builder, false, aliases);
     }
     interface BinaryFunctionBuilder<T> {
         T build(Location location, Expression lhs, Expression rhs);
     }
 
-    private static FunctionDefinition def(Class<? extends Function> function, FunctionBuilder builder, String... aliases) {
+    private static FunctionDefinition def(Class<? extends Function> function, FunctionBuilder builder,
+            boolean datetime, String... aliases) {
         String primaryName = normalize(function.getSimpleName());
-        BiFunction<UnresolvedFunction, DateTimeZone, Function> realBuilder = (uf, tz) -> {
+        FunctionDefinition.Builder realBuilder = (uf, distinct, tz) -> {
             try {
-                return builder.build(uf.location(), uf.children(), uf.distinct(), tz);
+                return builder.build(uf.location(), uf.children(), distinct, tz);
             } catch (IllegalArgumentException e) {
                 throw new ParsingException("error building [" + primaryName + "]: " + e.getMessage(), e,
                         uf.location().getLineNumber(), uf.location().getColumnNumber());
             }
         };
-        return new FunctionDefinition(primaryName, unmodifiableList(Arrays.asList(aliases)), function, realBuilder);
+        return new FunctionDefinition(primaryName, unmodifiableList(Arrays.asList(aliases)), function, datetime, realBuilder);
     }
     private interface FunctionBuilder {
         Function build(Location location, List<Expression> children, boolean distinct, DateTimeZone tz);

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/UnresolvedFunction.java

@@ -9,19 +9,30 @@ import org.elasticsearch.xpack.sql.capabilities.Unresolvable;
 import org.elasticsearch.xpack.sql.capabilities.UnresolvedException;
 import org.elasticsearch.xpack.sql.expression.Attribute;
 import org.elasticsearch.xpack.sql.expression.Expression;
+import org.elasticsearch.xpack.sql.expression.Literal;
 import org.elasticsearch.xpack.sql.tree.Location;
 import org.elasticsearch.xpack.sql.tree.NodeInfo;
 import org.elasticsearch.xpack.sql.type.DataType;
 import org.elasticsearch.xpack.sql.util.CollectionUtils;
-
+import org.elasticsearch.xpack.sql.util.StringUtils;
+import org.joda.time.DateTimeZone;
 import java.util.List;
+import java.util.Locale;
 import java.util.Objects;
+import java.util.Set;
+
+import static java.util.Collections.singletonList;
+
+import java.util.LinkedHashSet;
 
 public class UnresolvedFunction extends Function implements Unresolvable {
-
     private final String name;
-    private final boolean distinct;
     private final String unresolvedMsg;
+    /**
+     * How the resolution should be performed. This is changed depending
+     * on how the function was called.
+     */
+    private final ResolutionType resolutionType;
     /**
      * Flag to indicate analysis has been applied and there's no point in
      * doing it again this is an optimization to prevent searching for a
@@ -29,32 +40,70 @@ public class UnresolvedFunction extends Function implements Unresolvable {
      */
     private final boolean analyzed;
 
-    public UnresolvedFunction(Location location, String name, boolean distinct, List<Expression> children) {
+    public UnresolvedFunction(Location location, String name, ResolutionType resolutionType, List<Expression> children) {
-        this(location, name, distinct, children, false, null);
+        this(location, name, resolutionType, children, false, null);
     }
 
     /**
      * Constructor used for specifying a more descriptive message (typically
      * 'did you mean') instead of the default one.
+     * @see #withMessage(String)
      */
-    public UnresolvedFunction(Location location, String name, boolean distinct, List<Expression> children,
+    UnresolvedFunction(Location location, String name, ResolutionType resolutionType, List<Expression> children,
             boolean analyzed, String unresolvedMessage) {
         super(location, children);
         this.name = name;
-        this.distinct = distinct;
+        this.resolutionType = resolutionType;
         this.analyzed = analyzed;
-        this.unresolvedMsg = unresolvedMessage == null ? errorMessage(name, null) : unresolvedMessage;
+        this.unresolvedMsg = unresolvedMessage == null ? "Unknown " + resolutionType.type() + " [" + name + "]" : unresolvedMessage;
     }
 
     @Override
     protected NodeInfo<UnresolvedFunction> info() {
         return NodeInfo.create(this, UnresolvedFunction::new,
-            name, distinct, children(), analyzed, unresolvedMsg);
+            name, resolutionType, children(), analyzed, unresolvedMsg);
     }
 
     @Override
     public Expression replaceChildren(List<Expression> newChildren) {
-        return new UnresolvedFunction(location(), name, distinct, newChildren, analyzed, unresolvedMsg);
+        return new UnresolvedFunction(location(), name, resolutionType, newChildren, analyzed, unresolvedMsg);
+    }
+
+    public UnresolvedFunction withMessage(String message) {
+        return new UnresolvedFunction(location(), name(), resolutionType, children(), true, message);
+    }
+
+    public UnresolvedFunction preprocessStar() {
+        return resolutionType.preprocessStar(this);
+    }
+
+    /**
+     * Build a function to replace this one after resolving the function.
+     */
+    public Function buildResolved(DateTimeZone timeZone, FunctionDefinition def) {
+        return resolutionType.buildResolved(this, timeZone, def);
+    }
+
+    /**
+     * Build a marker {@link UnresolvedFunction} with an error message
+     * about the function being missing.
+     */
+    public UnresolvedFunction missing(String normalizedName, Iterable<FunctionDefinition> alternatives) {
+        // try to find alternatives
+        Set<String> names = new LinkedHashSet<>();
+        for (FunctionDefinition def : alternatives) {
+            if (resolutionType.isValidAlternative(def)) {
+                names.add(def.name());
+                names.addAll(def.aliases());
+            }
+        }
+
+        List<String> matches = StringUtils.findSimilar(normalizedName, names);
+        if (matches.isEmpty()) {
+            return this;
+        }
+        String matchesMessage = matches.size() == 1 ? "[" + matches.get(0) + "]" : "any of " + matches;
+        return withMessage("Unknown " + resolutionType.type() + " [" + name + "], did you mean " + matchesMessage + "?");
     }
 
     @Override
@@ -72,8 +121,8 @@ public class UnresolvedFunction extends Function implements Unresolvable {
         return name;
     }
 
-    public boolean distinct() {
+    ResolutionType resolutionType() {
-        return distinct;
+        return resolutionType;
     }
 
     public boolean analyzed() {
@@ -112,23 +161,119 @@ public class UnresolvedFunction extends Function implements Unresolvable {
         }
         UnresolvedFunction other = (UnresolvedFunction) obj;
         return name.equals(other.name)
-            && distinct == other.distinct
+            && resolutionType.equals(other.resolutionType)
             && children().equals(other.children())
             && analyzed == other.analyzed
-            && unresolvedMsg.equals(other.unresolvedMsg);
+            && Objects.equals(unresolvedMsg, other.unresolvedMsg);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(name, distinct, children(), analyzed, unresolvedMsg);
+        return Objects.hash(name, resolutionType, children(), analyzed, unresolvedMsg);
     }
 
-    public static String errorMessage(String name, List<String> potentialMatches) {
+    /**
-        String msg = "Unknown function [" + name + "]";
+     * Customize how function resolution works based on
-        if (!CollectionUtils.isEmpty(potentialMatches)) {
+     * where the function appeared in the grammar.
-            msg += ", did you mean "
+     */
-                    + (potentialMatches.size() == 1 ? "[" + potentialMatches.get(0) + "]" : "any of " + potentialMatches.toString()) + "?";
+    public enum ResolutionType {
-        }
+        /**
-        return msg;
+         * Behavior of standard function calls like {@code ABS(col)}.
+         */
+        STANDARD {
+            @Override
+            public UnresolvedFunction preprocessStar(UnresolvedFunction uf) {
+                // TODO: might be removed
+                // dedicated count optimization
+                if (uf.name.toUpperCase(Locale.ROOT).equals("COUNT")) {
+                    return new UnresolvedFunction(uf.location(), uf.name(), uf.resolutionType,
+                        singletonList(Literal.of(uf.arguments().get(0).location(), Integer.valueOf(1))));
+                }
+                return uf;
+            }
+            @Override
+            public Function buildResolved(UnresolvedFunction uf, DateTimeZone tz, FunctionDefinition def) {
+                return def.builder().build(uf, false, tz);
+            }
+            @Override
+            protected boolean isValidAlternative(FunctionDefinition def) {
+                return true;
+            }
+            @Override
+            protected String type() {
+                return "function";
+            }
+        },
+        /**
+         * Behavior of DISTINCT like {@code COUNT DISTINCT(col)}.
+         */
+        DISTINCT {
+            @Override
+            public UnresolvedFunction preprocessStar(UnresolvedFunction uf) {
+                return uf.withMessage("* is not valid with DISTINCT");
+            }
+            @Override
+            public Function buildResolved(UnresolvedFunction uf, DateTimeZone tz, FunctionDefinition def) {
+                return def.builder().build(uf, true, tz);
+            }
+            @Override
+            protected boolean isValidAlternative(FunctionDefinition def) {
+                return false; // think about this later.
+            }
+            @Override
+            protected String type() {
+                return "function";
+            }
+        },
+        /**
+         * Behavior of EXTRACT function calls like {@code EXTRACT(DAY FROM col)}.
+         */
+        EXTRACT {
+            @Override
+            public UnresolvedFunction preprocessStar(UnresolvedFunction uf) {
+                return uf.withMessage("Can't extract from *");
+            }
+            @Override
+            public Function buildResolved(UnresolvedFunction uf, DateTimeZone tz, FunctionDefinition def) {
+                if (def.datetime()) {
+                    return def.builder().build(uf, false, tz);
+                }
+                return uf.withMessage("Invalid datetime field [" + uf.name() + "]. Use any datetime function.");
+            }
+            @Override
+            protected boolean isValidAlternative(FunctionDefinition def) {
+                return def.datetime();
+            }
+            @Override
+            protected String type() {
+                return "datetime field";
+            }
+        };
+        /**
+         * Preprocess a function that contains a star to some other
+         * form before attempting to resolve it. For example,
+         * {@code DISTINCT} doesn't support {@code *} so it converts
+         * this function into a dead end, unresolveable function.
+         * Or {@code COUNT(*)} can be rewritten to {@code COUNT(1)}
+         * so we don't have to resolve {@code *}.
+         */
+        protected abstract UnresolvedFunction preprocessStar(UnresolvedFunction uf);
+        /**
+         * Build the real function from this one and resolution metadata.
+         */
+        protected abstract Function buildResolved(UnresolvedFunction uf, DateTimeZone tz, FunctionDefinition def);
+        /**
+         * Is {@code def} a valid alternative for function invocations
+         * of this kind. Used to filter the list of "did you mean"
+         * options sent back to the user when they specify a missing
+         * function.
+         */
+        protected abstract boolean isValidAlternative(FunctionDefinition def);
+        /**
+         * The name of the kind of thing being resolved. Used when
+         * building the error message sent back to the user when
+         * they specify a function that doesn't exist.
+         */
+        protected abstract String type();
     }
 }

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/Extract.java

@@ -1,112 +0,0 @@
-/*
- * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
- * or more contributor license agreements. Licensed under the Elastic License;
- * you may not use this file except in compliance with the Elastic License.
- */
-package org.elasticsearch.xpack.sql.expression.function.scalar.datetime;
-
-import org.elasticsearch.xpack.sql.expression.Expression;
-import org.elasticsearch.xpack.sql.tree.Location;
-import org.joda.time.DateTimeZone;
-
-public enum Extract {
-
-    YEAR {
-        @Override
-        public DateTimeFunction toFunction(Location source, Expression argument, DateTimeZone timeZone) {
-            return new Year(source, argument, timeZone);
-        }
-    },
-    MONTH {
-        @Override
-        public DateTimeFunction toFunction(Location source, Expression argument, DateTimeZone timeZone) {
-            return new MonthOfYear(source, argument, timeZone);
-        }
-    },
-    WEEK {
-        @Override
-        public DateTimeFunction toFunction(Location source, Expression argument, DateTimeZone timeZone) {
-            return new WeekOfYear(source, argument, timeZone);
-        }
-    },
-    DAY {
-        @Override
-        public DateTimeFunction toFunction(Location source, Expression argument, DateTimeZone timeZone) {
-            return new DayOfMonth(source, argument, timeZone);
-        }
-    },
-    DAY_OF_MONTH {
-        @Override
-        public DateTimeFunction toFunction(Location source, Expression argument, DateTimeZone timeZone) {
-            return DAY.toFunction(source, argument, timeZone);
-        }
-    },
-    DOM {
-        @Override
-        public DateTimeFunction toFunction(Location source, Expression argument, DateTimeZone timeZone) {
-            return DAY.toFunction(source, argument, timeZone);
-        }
-    },
-    DAY_OF_WEEK {
-        @Override
-        public DateTimeFunction toFunction(Location source, Expression argument, DateTimeZone timeZone) {
-            return new DayOfWeek(source, argument, timeZone);
-        }
-    },
-    DOW {
-        @Override
-        public DateTimeFunction toFunction(Location source, Expression argument, DateTimeZone timeZone) {
-            return DAY_OF_WEEK.toFunction(source, argument, timeZone);
-        }
-    },
-    DAY_OF_YEAR {
-        @Override
-        public DateTimeFunction toFunction(Location source, Expression argument, DateTimeZone timeZone) {
-            return new DayOfYear(source, argument, timeZone);
-        }
-    },
-    DOY {
-        @Override
-        public DateTimeFunction toFunction(Location source, Expression argument, DateTimeZone timeZone) {
-            return DAY_OF_YEAR.toFunction(source, argument, timeZone);
-        }
-    },
-    HOUR {
-        @Override
-        public DateTimeFunction toFunction(Location source, Expression argument, DateTimeZone timeZone) {
-            return new HourOfDay(source, argument, timeZone);
-        }
-    },
-    MINUTE {
-        @Override
-        public DateTimeFunction toFunction(Location source, Expression argument, DateTimeZone timeZone) {
-            return new MinuteOfHour(source, argument, timeZone);
-        }
-    },
-    MINUTE_OF_HOUR {
-        @Override
-        public DateTimeFunction toFunction(Location source, Expression argument, DateTimeZone timeZone) {
-            return MINUTE.toFunction(source, argument, timeZone);
-        }
-    },
-    MINUTE_OF_DAY {
-        @Override
-        public DateTimeFunction toFunction(Location source, Expression argument, DateTimeZone timeZone) {
-            return new MinuteOfDay(source, argument, timeZone);
-        }
-    },
-    SECOND {
-        @Override
-        public DateTimeFunction toFunction(Location source, Expression argument, DateTimeZone timeZone) {
-            return new SecondOfMinute(source, argument, timeZone);
-        }
-    },
-    SECOND_OF_MINUTE {
-        @Override
-        public DateTimeFunction toFunction(Location source, Expression argument, DateTimeZone timeZone) {
-            return SECOND.toFunction(source, argument, timeZone);
-        }
-    };
-
-    public abstract DateTimeFunction toFunction(Location source, Expression argument, DateTimeZone timeZone);
-}

plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/ExpressionBuilder.java

@@ -26,7 +26,6 @@ import org.elasticsearch.xpack.sql.expression.function.scalar.arithmetic.Mod;
 import org.elasticsearch.xpack.sql.expression.function.scalar.arithmetic.Mul;
 import org.elasticsearch.xpack.sql.expression.function.scalar.arithmetic.Neg;
 import org.elasticsearch.xpack.sql.expression.function.scalar.arithmetic.Sub;
-import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.Extract;
 import org.elasticsearch.xpack.sql.expression.predicate.And;
 import org.elasticsearch.xpack.sql.expression.predicate.Equals;
 import org.elasticsearch.xpack.sql.expression.predicate.GreaterThan;
@@ -75,19 +74,20 @@ import org.elasticsearch.xpack.sql.parser.SqlBaseParser.StringQueryContext;
 import org.elasticsearch.xpack.sql.parser.SqlBaseParser.SubqueryExpressionContext;
 import org.elasticsearch.xpack.sql.tree.Location;
 import org.elasticsearch.xpack.sql.type.DataType;
-import org.elasticsearch.xpack.sql.type.DataTypes;
 import org.joda.time.DateTimeZone;
 
 import java.math.BigDecimal;
 import java.util.List;
 import java.util.Locale;
 
+import static java.util.Collections.singletonList;
+
 abstract class ExpressionBuilder extends IdentifierBuilder {
     /**
      * Time zone that we're executing in. Set on functions that deal
      * with dates and times for use later in the evaluation process.
      */
-    private final DateTimeZone timeZone;
+    private final DateTimeZone timeZone; // TODO remove me
 
     protected ExpressionBuilder(DateTimeZone timeZone) {
         this.timeZone = timeZone;
@@ -366,25 +366,17 @@ abstract class ExpressionBuilder extends IdentifierBuilder {
     @Override
     public Object visitFunctionCall(FunctionCallContext ctx) {
         String name = visitIdentifier(ctx.identifier());
-        boolean isDistinct = false;
+        boolean isDistinct = ctx.setQuantifier() != null && ctx.setQuantifier().DISTINCT() != null;
-        if (ctx.setQuantifier() != null) {
+        UnresolvedFunction.ResolutionType resolutionType =
-            isDistinct = ctx.setQuantifier().DISTINCT() != null;
+                isDistinct ? UnresolvedFunction.ResolutionType.DISTINCT : UnresolvedFunction.ResolutionType.STANDARD;
-        }
+        return new UnresolvedFunction(source(ctx), name, resolutionType, expressions(ctx.expression()));
-
-        return new UnresolvedFunction(source(ctx), name, isDistinct, expressions(ctx.expression()));
     }
 
     @Override
     public Object visitExtract(ExtractContext ctx) {
-        Location source = source(ctx);
         String fieldString = visitIdentifier(ctx.field);
-        Extract extract = null;
+        return new UnresolvedFunction(source(ctx), fieldString,
-        try {
+                UnresolvedFunction.ResolutionType.EXTRACT, singletonList(expression(ctx.valueExpression())));
-            extract = Extract.valueOf(fieldString.toUpperCase(Locale.ROOT));
-        } catch (IllegalArgumentException ex) {
-            throw new ParsingException(source, "Invalid EXTRACT field {}", fieldString);
-        }
-        return extract.toFunction(source, expression(ctx.valueExpression()), timeZone);
     }
 
     @Override

plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java

@@ -76,6 +76,23 @@ public class VerifierErrorMessagesTests extends ESTestCase {
         assertEquals("1:41: Unknown column [xxx]", verify("SELECT * FROM test ORDER BY DAY_oF_YEAR(xxx)"));
     }
 
+    public void testMissingExtract() {
+        assertEquals("1:8: Unknown datetime field [ZAZ]", verify("SELECT EXTRACT(ZAZ FROM date) FROM test"));
+    }
+
+    public void testMissingExtractSimilar() {
+        assertEquals("1:8: Unknown datetime field [DAP], did you mean [DAY]?", verify("SELECT EXTRACT(DAP FROM date) FROM test"));
+    }
+
+    public void testMissingExtractSimilarMany() {
+        assertEquals("1:8: Unknown datetime field [DOP], did you mean any of [DOM, DOW, DOY]?",
+            verify("SELECT EXTRACT(DOP FROM date) FROM test"));
+    }
+
+    public void testExtractNonDateTime() {
+        assertEquals("1:8: Invalid datetime field [ABS]. Use any datetime function.", verify("SELECT EXTRACT(ABS FROM date) FROM test"));
+    }
+
     public void testMultipleColumns() {
         // xxx offset is that of the order by field
         assertEquals("1:43: Unknown column [xxx]\nline 1:8: Unknown column [xxx]",
@@ -127,4 +144,4 @@ public class VerifierErrorMessagesTests extends ESTestCase {
         assertEquals("1:8: Cannot use field [unsupported] type [ip_range] as is unsupported",
                 verify("SELECT unsupported FROM test"));
     }
-}
+}

plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/UnresolvedAttributeTests.java

@@ -0,0 +1,125 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+/*
+* ELASTICSEARCH CONFIDENTIAL
+* __________________
+*
+*  [2017] Elasticsearch Incorporated. All Rights Reserved.
+*
+* NOTICE:  All information contained herein is, and remains
+* the property of Elasticsearch Incorporated and its suppliers,
+* if any.  The intellectual and technical concepts contained
+* herein are proprietary to Elasticsearch Incorporated
+* and its suppliers and may be covered by U.S. and Foreign Patents,
+* patents in process, and are protected by trade secret or copyright law.
+* Dissemination of this information or reproduction of this material
+* is strictly forbidden unless prior written permission is obtained
+* from Elasticsearch Incorporated.
+*/
+
+package org.elasticsearch.xpack.sql.expression;
+
+import org.elasticsearch.xpack.sql.tree.AbstractNodeTestCase;
+import org.elasticsearch.xpack.sql.tree.Location;
+
+import java.util.Arrays;
+import java.util.Objects;
+import java.util.function.Supplier;
+
+import static org.elasticsearch.xpack.sql.tree.LocationTests.randomLocation;
+
+public class UnresolvedAttributeTests extends AbstractNodeTestCase<UnresolvedAttribute, Expression> {
+    public static UnresolvedAttribute randomUnresolvedAttribute() {
+        Location location = randomLocation();
+        String name = randomAlphaOfLength(5);
+        String qualifier = randomQualifier();
+        ExpressionId id = randomBoolean() ? null : new ExpressionId();
+        String unresolvedMessage = randomUnresolvedMessage();
+        Object resolutionMetadata = new Object();
+        return new UnresolvedAttribute(location, name, qualifier, id, unresolvedMessage, resolutionMetadata);
+    }
+
+    /**
+     * A random qualifier. It is important that this be distinct
+     * from the name and the unresolvedMessage for testing transform.
+     */
+    private static String randomQualifier() {
+        return randomBoolean() ? null : randomAlphaOfLength(6);
+    }
+
+    /**
+     * A random qualifier. It is important that this be distinct
+     * from the name and the qualifier for testing transform.
+     */
+    private static String randomUnresolvedMessage() {
+        return randomAlphaOfLength(7);
+    }
+
+    @Override
+    protected UnresolvedAttribute randomInstance() {
+        return randomUnresolvedAttribute();
+    }
+
+    @Override
+    protected UnresolvedAttribute mutate(UnresolvedAttribute a) {
+        Supplier<UnresolvedAttribute> option = randomFrom(Arrays.asList(
+            () -> new UnresolvedAttribute(a.location(),
+                    randomValueOtherThan(a.name(), () -> randomAlphaOfLength(5)),
+                    a.qualifier(), a.id(), a.unresolvedMessage(), a.resolutionMetadata()),
+            () -> new UnresolvedAttribute(a.location(), a.name(),
+                    randomValueOtherThan(a.qualifier(), UnresolvedAttributeTests::randomQualifier),
+                    a.id(), a.unresolvedMessage(), a.resolutionMetadata()),
+            () -> new UnresolvedAttribute(a.location(), a.name(), a.qualifier(),
+                    new ExpressionId(), a.unresolvedMessage(), a.resolutionMetadata()),
+            () -> new UnresolvedAttribute(a.location(), a.name(), a.qualifier(), a.id(),
+                    randomValueOtherThan(a.unresolvedMessage(), () -> randomUnresolvedMessage()),
+                    a.resolutionMetadata()),
+            () -> new UnresolvedAttribute(a.location(), a.name(),
+                    a.qualifier(), a.id(), a.unresolvedMessage(), new Object())
+        ));
+        return option.get();
+    }
+
+    @Override
+    protected UnresolvedAttribute copy(UnresolvedAttribute a) {
+        return new UnresolvedAttribute(a.location(), a.name(), a.qualifier(), a.id(), a.unresolvedMessage(), a.resolutionMetadata());
+    }
+
+    @Override
+    public void testTransform() {
+        UnresolvedAttribute a = randomUnresolvedAttribute();
+
+        String newName = randomValueOtherThan(a.name(), () -> randomAlphaOfLength(5));
+        assertEquals(new UnresolvedAttribute(a.location(), newName, a.qualifier(), a.id(),
+                a.unresolvedMessage(), a.resolutionMetadata()),
+            a.transformPropertiesOnly(v -> Objects.equals(v, a.name()) ? newName : v, Object.class));
+
+        String newQualifier = randomValueOtherThan(a.qualifier(), UnresolvedAttributeTests::randomQualifier);
+        assertEquals(new UnresolvedAttribute(a.location(), a.name(), newQualifier, a.id(),
+                a.unresolvedMessage(), a.resolutionMetadata()),
+            a.transformPropertiesOnly(v -> Objects.equals(v, a.qualifier()) ? newQualifier : v, Object.class));
+
+        ExpressionId newId = new ExpressionId();
+        assertEquals(new UnresolvedAttribute(a.location(), a.name(), a.qualifier(), newId,
+                a.unresolvedMessage(), a.resolutionMetadata()),
+            a.transformPropertiesOnly(v -> Objects.equals(v, a.id()) ? newId : v, Object.class));
+
+        String newMessage = randomValueOtherThan(a.unresolvedMessage(), UnresolvedAttributeTests::randomUnresolvedMessage);
+        assertEquals(new UnresolvedAttribute(a.location(), a.name(), a.qualifier(), a.id(),
+                newMessage, a.resolutionMetadata()),
+            a.transformPropertiesOnly(v -> Objects.equals(v, a.unresolvedMessage()) ? newMessage : v, Object.class));
+
+        Object newMeta = new Object();
+        assertEquals(new UnresolvedAttribute(a.location(), a.name(), a.qualifier(), a.id(),
+                a.unresolvedMessage(), newMeta),
+            a.transformPropertiesOnly(v -> Objects.equals(v, a.resolutionMetadata()) ? newMeta : v, Object.class));
+    }
+
+    @Override
+    public void testReplaceChildren() {
+        // UnresolvedAttribute doesn't have any children
+    }
+}

plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/FunctionRegistryTests.java

@@ -20,129 +20,144 @@ import java.util.Arrays;
 import java.util.List;
 
 import static org.elasticsearch.xpack.sql.expression.function.FunctionRegistry.def;
+import static org.elasticsearch.xpack.sql.expression.function.UnresolvedFunction.ResolutionType.DISTINCT;
+import static org.elasticsearch.xpack.sql.expression.function.UnresolvedFunction.ResolutionType.EXTRACT;
+import static org.elasticsearch.xpack.sql.expression.function.UnresolvedFunction.ResolutionType.STANDARD;
 import static org.hamcrest.Matchers.endsWith;
 import static org.mockito.Mockito.mock;
 import static java.util.Collections.emptyList;
 
 public class FunctionRegistryTests extends ESTestCase {
     public void testNoArgFunction() {
-        UnresolvedFunction ur = uf(false);
+        UnresolvedFunction ur = uf(STANDARD);
         FunctionRegistry r = new FunctionRegistry(Arrays.asList(def(Dummy.class, Dummy::new)));
-        assertEquals(ur.location(), r.resolveFunction(ur, randomDateTimeZone()).location());
+        FunctionDefinition def = r.resolveFunction(ur.name());
+        assertEquals(ur.location(), ur.buildResolved(randomDateTimeZone(), def).location());
 
         // Distinct isn't supported
         ParsingException e = expectThrows(ParsingException.class, () ->
-            r.resolveFunction(uf(true), randomDateTimeZone()));
+                uf(DISTINCT).buildResolved(randomDateTimeZone(), def));
         assertThat(e.getMessage(), endsWith("does not support DISTINCT yet it was specified"));
 
         // Any children aren't supported
         e = expectThrows(ParsingException.class, () ->
-            r.resolveFunction(uf(false, mock(Expression.class)), randomDateTimeZone()));
+                uf(STANDARD, mock(Expression.class)).buildResolved(randomDateTimeZone(), def));
         assertThat(e.getMessage(), endsWith("expects no arguments"));
     }
 
     public void testUnaryFunction() {
-        UnresolvedFunction ur = uf(false, mock(Expression.class));
+        UnresolvedFunction ur = uf(STANDARD, mock(Expression.class));
         FunctionRegistry r = new FunctionRegistry(Arrays.asList(def(Dummy.class, (Location l, Expression e) -> {
             assertSame(e, ur.children().get(0));
             return new Dummy(l);
         })));
-        assertEquals(ur.location(), r.resolveFunction(ur, randomDateTimeZone()).location());
+        FunctionDefinition def = r.resolveFunction(ur.name());
+        assertFalse(def.datetime());
+        assertEquals(ur.location(), ur.buildResolved(randomDateTimeZone(), def).location());
 
         // Distinct isn't supported
         ParsingException e = expectThrows(ParsingException.class, () ->
-            r.resolveFunction(uf(true, mock(Expression.class)), randomDateTimeZone()));
+                uf(DISTINCT, mock(Expression.class)).buildResolved(randomDateTimeZone(), def));
         assertThat(e.getMessage(), endsWith("does not support DISTINCT yet it was specified"));
 
         // No children aren't supported
         e = expectThrows(ParsingException.class, () ->
-            r.resolveFunction(uf(false), randomDateTimeZone()));
+                uf(STANDARD).buildResolved(randomDateTimeZone(), def));
         assertThat(e.getMessage(), endsWith("expects exactly one argument"));
 
         // Multiple children aren't supported
         e = expectThrows(ParsingException.class, () ->
-            r.resolveFunction(uf(false, mock(Expression.class), mock(Expression.class)), randomDateTimeZone()));
+            uf(STANDARD, mock(Expression.class), mock(Expression.class)).buildResolved(randomDateTimeZone(), def));
         assertThat(e.getMessage(), endsWith("expects exactly one argument"));
     }
 
     public void testUnaryDistinctAwareFunction() {
-        UnresolvedFunction ur = uf(randomBoolean(), mock(Expression.class));
+        boolean urIsDistinct = randomBoolean();
+        UnresolvedFunction ur = uf(urIsDistinct ? DISTINCT : STANDARD, mock(Expression.class));
         FunctionRegistry r = new FunctionRegistry(Arrays.asList(def(Dummy.class, (Location l, Expression e, boolean distinct) -> {
-            assertEquals(ur.distinct(), distinct);
+            assertEquals(urIsDistinct, distinct);
             assertSame(e, ur.children().get(0));
             return new Dummy(l);
         })));
-        assertEquals(ur.location(), r.resolveFunction(ur, randomDateTimeZone()).location());
+        FunctionDefinition def = r.resolveFunction(ur.name());
+        assertEquals(ur.location(), ur.buildResolved(randomDateTimeZone(), def).location());
+        assertFalse(def.datetime());
 
         // No children aren't supported
         ParsingException e = expectThrows(ParsingException.class, () ->
-            r.resolveFunction(uf(false), randomDateTimeZone()));
+                uf(STANDARD).buildResolved(randomDateTimeZone(), def));
         assertThat(e.getMessage(), endsWith("expects exactly one argument"));
 
         // Multiple children aren't supported
         e = expectThrows(ParsingException.class, () ->
-            r.resolveFunction(uf(false, mock(Expression.class), mock(Expression.class)), randomDateTimeZone()));
+                uf(STANDARD, mock(Expression.class), mock(Expression.class)).buildResolved(randomDateTimeZone(), def));
         assertThat(e.getMessage(), endsWith("expects exactly one argument"));
     }
 
-    public void testTimeZoneAwareFunction() {
+    public void testDateTimeFunction() {
-        UnresolvedFunction ur = uf(false, mock(Expression.class));
+        boolean urIsExtract = randomBoolean();
+        UnresolvedFunction ur = uf(urIsExtract ? EXTRACT : STANDARD, mock(Expression.class));
         DateTimeZone providedTimeZone = randomDateTimeZone();
         FunctionRegistry r = new FunctionRegistry(Arrays.asList(def(Dummy.class, (Location l, Expression e, DateTimeZone tz) -> {
             assertEquals(providedTimeZone, tz);
             assertSame(e, ur.children().get(0));
             return new Dummy(l);
         })));
-        assertEquals(ur.location(), r.resolveFunction(ur, providedTimeZone).location());
+        FunctionDefinition def = r.resolveFunction(ur.name());
+        assertEquals(ur.location(), ur.buildResolved(providedTimeZone, def).location());
+        assertTrue(def.datetime());
 
         // Distinct isn't supported
         ParsingException e = expectThrows(ParsingException.class, () ->
-            r.resolveFunction(uf(true, mock(Expression.class)), randomDateTimeZone()));
+                uf(DISTINCT, mock(Expression.class)).buildResolved(randomDateTimeZone(), def));
         assertThat(e.getMessage(), endsWith("does not support DISTINCT yet it was specified"));
 
         // No children aren't supported
         e = expectThrows(ParsingException.class, () ->
-            r.resolveFunction(uf(false), randomDateTimeZone()));
+                uf(STANDARD).buildResolved(randomDateTimeZone(), def));
         assertThat(e.getMessage(), endsWith("expects exactly one argument"));
 
         // Multiple children aren't supported
         e = expectThrows(ParsingException.class, () ->
-            r.resolveFunction(uf(false, mock(Expression.class), mock(Expression.class)), randomDateTimeZone()));
+                uf(STANDARD, mock(Expression.class), mock(Expression.class)).buildResolved(randomDateTimeZone(), def));
         assertThat(e.getMessage(), endsWith("expects exactly one argument"));
     }
 
     public void testBinaryFunction() {
-        UnresolvedFunction ur = uf(false, mock(Expression.class), mock(Expression.class));
+        UnresolvedFunction ur = uf(STANDARD, mock(Expression.class), mock(Expression.class));
         FunctionRegistry r = new FunctionRegistry(Arrays.asList(def(Dummy.class, (Location l, Expression lhs, Expression rhs) -> {
             assertSame(lhs, ur.children().get(0));
             assertSame(rhs, ur.children().get(1));
             return new Dummy(l);
         })));
-        assertEquals(ur.location(), r.resolveFunction(ur, randomDateTimeZone()).location());
+        FunctionDefinition def = r.resolveFunction(ur.name());
+        assertEquals(ur.location(), ur.buildResolved(randomDateTimeZone(), def).location());
+        assertFalse(def.datetime());
 
         // Distinct isn't supported
         ParsingException e = expectThrows(ParsingException.class, () ->
-            r.resolveFunction(uf(true, mock(Expression.class), mock(Expression.class)), randomDateTimeZone()));
+                uf(DISTINCT, mock(Expression.class), mock(Expression.class)).buildResolved(randomDateTimeZone(), def));
         assertThat(e.getMessage(), endsWith("does not support DISTINCT yet it was specified"));
 
         // No children aren't supported
         e = expectThrows(ParsingException.class, () ->
-            r.resolveFunction(uf(false), randomDateTimeZone()));
+                uf(STANDARD).buildResolved(randomDateTimeZone(), def));
         assertThat(e.getMessage(), endsWith("expects exactly two arguments"));
 
         // One child isn't supported
         e = expectThrows(ParsingException.class, () ->
-            r.resolveFunction(uf(false, mock(Expression.class)), randomDateTimeZone()));
+                uf(STANDARD, mock(Expression.class)).buildResolved(randomDateTimeZone(), def));
         assertThat(e.getMessage(), endsWith("expects exactly two arguments"));
 
         // Many children aren't supported
         e = expectThrows(ParsingException.class, () ->
-            r.resolveFunction(uf(false, mock(Expression.class), mock(Expression.class), mock(Expression.class)), randomDateTimeZone()));
+                uf(STANDARD, mock(Expression.class), mock(Expression.class), mock(Expression.class))
+                    .buildResolved(randomDateTimeZone(), def));
         assertThat(e.getMessage(), endsWith("expects exactly two arguments"));
     }
 
-    private UnresolvedFunction uf(boolean distinct, Expression... children) {
+    private UnresolvedFunction uf(UnresolvedFunction.ResolutionType resolutionType, Expression... children) {
-        return new UnresolvedFunction(LocationTests.randomLocation(), "dummy", distinct, Arrays.asList(children));
+        return new UnresolvedFunction(LocationTests.randomLocation(), "dummy", resolutionType, Arrays.asList(children));
     }
 
     public static class Dummy extends ScalarFunction {

plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/UnresolvedFunctionTests.java

@@ -0,0 +1,141 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+/*
+* ELASTICSEARCH CONFIDENTIAL
+* __________________
+*
+*  [2017] Elasticsearch Incorporated. All Rights Reserved.
+*
+* NOTICE:  All information contained herein is, and remains
+* the property of Elasticsearch Incorporated and its suppliers,
+* if any.  The intellectual and technical concepts contained
+* herein are proprietary to Elasticsearch Incorporated
+* and its suppliers and may be covered by U.S. and Foreign Patents,
+* patents in process, and are protected by trade secret or copyright law.
+* Dissemination of this information or reproduction of this material
+* is strictly forbidden unless prior written permission is obtained
+* from Elasticsearch Incorporated.
+*/
+
+package org.elasticsearch.xpack.sql.expression.function;
+
+import org.elasticsearch.xpack.sql.expression.Expression;
+import org.elasticsearch.xpack.sql.expression.function.UnresolvedFunction;
+import org.elasticsearch.xpack.sql.tree.AbstractNodeTestCase;
+import org.elasticsearch.xpack.sql.tree.Location;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+import java.util.function.Supplier;
+
+import static org.elasticsearch.xpack.sql.tree.LocationTests.randomLocation;
+import static org.elasticsearch.xpack.sql.expression.UnresolvedAttributeTests.randomUnresolvedAttribute;
+
+import static java.util.Collections.singletonList;
+
+public class UnresolvedFunctionTests extends AbstractNodeTestCase<UnresolvedFunction, Expression> {
+    public static UnresolvedFunction randomUnresolvedFunction() {
+        /* Pick an UnresolvedFunction where the name and the
+         * message don't happen to be the same String. If they
+         * matched then transform would get them confused. */
+        Location location = randomLocation();
+        String name = randomAlphaOfLength(5);
+        UnresolvedFunction.ResolutionType resolutionType = randomFrom(UnresolvedFunction.ResolutionType.values());
+        List<Expression> args = randomFunctionArgs();
+        boolean analyzed = randomBoolean();
+        String unresolvedMessage = randomUnresolvedMessage();
+        return new UnresolvedFunction(location, name, resolutionType, args, analyzed, unresolvedMessage);
+    }
+
+    private static List<Expression> randomFunctionArgs() {
+        // At this point we only support functions with 0, 1, or 2 arguments.
+        Supplier<List<Expression>> option = randomFrom(Arrays.asList(
+            Collections::emptyList,
+            () -> singletonList(randomUnresolvedAttribute()),
+            () -> Arrays.asList(randomUnresolvedAttribute(), randomUnresolvedAttribute())
+        ));
+        return option.get();
+    }
+
+    /**
+     * Pick a random value for the unresolved message.
+     * It is important that this value is not the same
+     * as the value for the name for tests like the {@link #testTransform}
+     * and for general ease of reading.
+     */
+    private static String randomUnresolvedMessage() {
+        return randomBoolean() ? null : randomAlphaOfLength(6);
+    }
+
+    @Override
+    protected UnresolvedFunction randomInstance() {
+        return randomUnresolvedFunction();
+    }
+
+    @Override
+    protected UnresolvedFunction mutate(UnresolvedFunction uf) {
+        Supplier<UnresolvedFunction> option = randomFrom(Arrays.asList(
+            () -> new UnresolvedFunction(uf.location(), randomValueOtherThan(uf.name(), () -> randomAlphaOfLength(5)),
+                    uf.resolutionType(), uf.children(), uf.analyzed(), uf.unresolvedMessage()),
+            () -> new UnresolvedFunction(uf.location(), uf.name(),
+                    randomValueOtherThan(uf.resolutionType(), () -> randomFrom(UnresolvedFunction.ResolutionType.values())),
+                    uf.children(), uf.analyzed(), uf.unresolvedMessage()),
+            () -> new UnresolvedFunction(uf.location(), uf.name(), uf.resolutionType(),
+                    randomValueOtherThan(uf.children(), UnresolvedFunctionTests::randomFunctionArgs),
+                    uf.analyzed(), uf.unresolvedMessage()),
+            () -> new UnresolvedFunction(uf.location(), uf.name(), uf.resolutionType(), uf.children(),
+                    !uf.analyzed(), uf.unresolvedMessage()),
+            () -> new UnresolvedFunction(uf.location(), uf.name(), uf.resolutionType(), uf.children(),
+                    uf.analyzed(), randomValueOtherThan(uf.unresolvedMessage(), () -> randomAlphaOfLength(5)))
+        ));
+        return option.get();
+    }
+
+    @Override
+    protected UnresolvedFunction copy(UnresolvedFunction uf) {
+        return new UnresolvedFunction(uf.location(), uf.name(), uf.resolutionType(), uf.children(),
+                uf.analyzed(), uf.unresolvedMessage());
+    }
+
+    @Override
+    public void testTransform() {
+        UnresolvedFunction uf = randomUnresolvedFunction();
+
+        String newName = randomValueOtherThan(uf.name(), () -> randomAlphaOfLength(5));
+        assertEquals(new UnresolvedFunction(uf.location(), newName, uf.resolutionType(), uf.children(),
+                    uf.analyzed(), uf.unresolvedMessage()),
+                uf.transformPropertiesOnly(p -> Objects.equals(p, uf.name()) ? newName : p, Object.class));
+
+        UnresolvedFunction.ResolutionType newResolutionType = randomValueOtherThan(uf.resolutionType(),
+                () -> randomFrom(UnresolvedFunction.ResolutionType.values()));
+        assertEquals(new UnresolvedFunction(uf.location(), uf.name(), newResolutionType, uf.children(),
+                    uf.analyzed(), uf.unresolvedMessage()),
+                uf.transformPropertiesOnly(p -> Objects.equals(p, uf.resolutionType()) ? newResolutionType : p, Object.class));
+
+        String newUnresolvedMessage = randomValueOtherThan(uf.unresolvedMessage(),
+                UnresolvedFunctionTests::randomUnresolvedMessage);
+        assertEquals(new UnresolvedFunction(uf.location(), uf.name(), uf.resolutionType(), uf.children(),
+                    uf.analyzed(), newUnresolvedMessage),
+                uf.transformPropertiesOnly(p -> Objects.equals(p, uf.unresolvedMessage()) ? newUnresolvedMessage : p, Object.class));
+
+        assertEquals(new UnresolvedFunction(uf.location(), uf.name(), uf.resolutionType(), uf.children(),
+                !uf.analyzed(), uf.unresolvedMessage()),
+            uf.transformPropertiesOnly(p -> Objects.equals(p, uf.analyzed()) ? !uf.analyzed() : p, Object.class));
+
+    }
+
+    @Override
+    public void testReplaceChildren() {
+        UnresolvedFunction uf = randomUnresolvedFunction();
+
+        List<Expression> newChildren = randomValueOtherThan(uf.children(), UnresolvedFunctionTests::randomFunctionArgs);
+        assertEquals(new UnresolvedFunction(uf.location(), uf.name(), uf.resolutionType(), newChildren,
+                    uf.analyzed(), uf.unresolvedMessage()),
+                uf.replaceChildren(newChildren));
+    }
+}

plugin/sql/src/test/java/org/elasticsearch/xpack/sql/tree/NodeSubclassTests.java

@@ -12,7 +12,7 @@ import org.elasticsearch.common.io.PathUtils;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.xpack.sql.expression.Expression;
 import org.elasticsearch.xpack.sql.expression.FieldAttribute;
-import org.elasticsearch.xpack.sql.expression.UnresolvedAttribute;
+import org.elasticsearch.xpack.sql.expression.UnresolvedAttributeTests;
 import org.elasticsearch.xpack.sql.expression.function.Function;
 import org.elasticsearch.xpack.sql.expression.function.aggregate.AggregateFunction;
 import org.elasticsearch.xpack.sql.expression.function.aggregate.Avg;
@@ -441,7 +441,7 @@ public class NodeSubclassTests<T extends B, B extends Node<B>> extends ESTestCas
              * use a simple one. Without this we're very prone to
              * stackoverflow errors while building the tree.
              */
-            return new UnresolvedAttribute(LocationTests.randomLocation(), randomAlphaOfLength(5));
+            return UnresolvedAttributeTests.randomUnresolvedAttribute();
         }
         if (Node.class.isAssignableFrom(argClass)) {
             /*
@@ -502,7 +502,7 @@ public class NodeSubclassTests<T extends B, B extends Node<B>> extends ESTestCas
 
     }
 
-    private static <T extends Node<?>> T makeNode(Class<? extends T> nodeClass) throws Exception {
+    public static <T extends Node<?>> T makeNode(Class<? extends T> nodeClass) throws Exception {
         if (Modifier.isAbstract(nodeClass.getModifiers())) {
             nodeClass = randomFrom(subclassesOf(nodeClass));
         }

plugin/sql/src/test/resources/mapping-multi-field-with-nested.json

@@ -5,6 +5,7 @@
         "text" : { "type" : "text" },
         "keyword" : { "type" : "keyword" },
         "unsupported" : { "type" : "ip_range" },
+        "date" : { "type" : "date"},
         "some" : {
             "properties" : {
                 "dotted" : {