SQL: Remove reflection from constructing Functions (elastic/x-pack-elasticsearch#3442)

This replaces the marker interfaces and reflection that we once used to
construct functions with method references to constructors. It uses a
few overloaded methods to build the `FunctionDefinition`s from the
method references that adapt the various forms of `Function`
constructors that we have into
`BiFunction<UnresolvedFunction, DateTimeZone, Function>` so that the
compiler can do the complex task of picking the appropriate adapter. It
is good at that sort of thing.

Many of these overloaded functions have `@SuppressWarnings("overloads")`
because they are ambiguous if you wrote one as a lambda without type
parameters. Since we always use constructor references this isn't a
problem.

It does not remove the reflection from function naming or from function
type derivation. This is big enough and these seemed significantly less
fraught.

Original commit: elastic/x-pack-elasticsearch@528d05754b
This commit is contained in:
Nik Everett 2017-12-29 12:36:09 -05:00 committed by GitHub
parent 9f71100bac
commit dbf1fc00ce
9 changed files with 212 additions and 233 deletions

View File

@ -7,62 +7,56 @@ package org.elasticsearch.xpack.sql.expression.function;
import org.elasticsearch.common.Strings;
import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
import org.elasticsearch.xpack.sql.expression.function.aware.DistinctAware;
import org.elasticsearch.xpack.sql.expression.function.aware.TimeZoneAware;
import org.elasticsearch.xpack.sql.expression.Expression;
import org.elasticsearch.xpack.sql.parser.ParsingException;
import org.elasticsearch.xpack.sql.tree.Node;
import org.elasticsearch.xpack.sql.tree.NodeUtils;
import org.elasticsearch.xpack.sql.tree.NodeUtils.NodeInfo;
import org.elasticsearch.xpack.sql.util.Check;
import org.elasticsearch.xpack.sql.tree.Location;
import org.elasticsearch.xpack.sql.util.StringUtils;
import org.joda.time.DateTimeZone;
import java.lang.reflect.InvocationTargetException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.function.BiFunction;
import java.util.regex.Pattern;
import static java.util.Collections.emptyList;
import static java.util.Collections.unmodifiableList;
import static java.util.stream.Collectors.toList;
abstract class AbstractFunctionRegistry implements FunctionRegistry {
private final Map<String, FunctionDefinition> defs = new LinkedHashMap<>();
private final Map<String, String> aliases;
protected final Map<String, FunctionDefinition> defs = new LinkedHashMap<>();
{
for (Class<? extends Function> f : functions()) {
FunctionDefinition def = def(f, aliases());
defs.put(def.name(), def);
for (String alias : def.aliases()) {
Check.isTrue(defs.containsKey(alias) == false, "Alias %s already exists", alias);
// alias should be already normalized but to be double sure
defs.put(normalize(alias), def);
protected AbstractFunctionRegistry(List<FunctionDefinition> functions) {
this.aliases = new HashMap<>();
for (FunctionDefinition f : functions) {
defs.put(f.name(), f);
for (String alias : f.aliases()) {
Object old = aliases.put(alias, f.name());
if (old != null) {
throw new IllegalArgumentException("alias [" + alias + "] is used by [" + old + "] and [" + f.name() + "]");
}
defs.put(alias, f);
}
}
}
//TODO: change this to some type of auto discovery or auto creation of the discovery (annotation or the like)
protected abstract Collection<Class<? extends Function>> functions();
protected abstract Map<String, String> aliases();
@Override
public Function resolveFunction(UnresolvedFunction ur, DateTimeZone timeZone) {
FunctionDefinition def = defs.get(normalize(ur.name()));
if (def == null) {
throw new SqlIllegalArgumentException("Cannot find function %s; this should have been caught during analysis", ur.name());
}
return createInstance(def.clazz(), ur, timeZone);
return def.builder().apply(ur, timeZone);
}
@Override
public String concreteFunctionName(String alias) {
String normalized = normalize(alias);
return aliases().getOrDefault(normalized, normalized);
return aliases.getOrDefault(normalized, normalized);
}
@Override
@ -73,7 +67,7 @@ abstract class AbstractFunctionRegistry implements FunctionRegistry {
@Override
public Collection<FunctionDefinition> listFunctions() {
return defs.entrySet().stream()
.map(e -> new FunctionDefinition(e.getKey(), emptyList(), e.getValue().clazz()))
.map(e -> new FunctionDefinition(e.getKey(), emptyList(), e.getValue().clazz(), e.getValue().builder()))
.collect(toList());
}
@ -82,101 +76,128 @@ abstract class AbstractFunctionRegistry implements FunctionRegistry {
Pattern p = Strings.hasText(pattern) ? StringUtils.likeRegex(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()))
.map(e -> new FunctionDefinition(e.getKey(), emptyList(), e.getValue().clazz(), e.getValue().builder()))
.collect(toList());
}
private static FunctionDefinition def(Class<? extends Function> function, Map<String, String> aliases) {
String primaryName = normalize(function.getSimpleName());
List<String> al = aliases.entrySet().stream()
.filter(e -> primaryName.equals(e.getValue()))
.map(Map.Entry::getKey)
.collect(toList());
/**
* Build a {@linkplain FunctionDefinition} for a no-argument function that
* is not aware of time zone and does not support {@code DISTINCT}.
*/
protected static <T extends Function> FunctionDefinition def(Class<T> function,
java.util.function.Function<Location, T> ctorRef, String... aliases) {
FunctionBuilder builder = (location, children, distinct, tz) -> {
if (false == children.isEmpty()) {
throw new IllegalArgumentException("expects only a single argument");
}
if (distinct) {
throw new IllegalArgumentException("does not support DISTINCT yet it was specified");
}
return ctorRef.apply(location);
};
return def(function, builder, aliases);
}
return new FunctionDefinition(primaryName, al, function);
/**
* Build a {@linkplain FunctionDefinition} for a unary function that is not
* aware of time zone and does not support {@code DISTINCT}.
*/
@SuppressWarnings("overloads") // These are ambiguous if you aren't using ctor references but we always do
protected static <T extends Function> FunctionDefinition def(Class<T> function,
BiFunction<Location, Expression, T> ctorRef, String... aliases) {
FunctionBuilder builder = (location, children, distinct, tz) -> {
if (children.size() != 1) {
throw new IllegalArgumentException("expects only a single argument");
}
if (distinct) {
throw new IllegalArgumentException("does not support DISTINCT yet it was specified");
}
return ctorRef.apply(location, children.get(0));
};
return def(function, builder, aliases);
}
/**
* Build a {@linkplain FunctionDefinition} for a unary function that is not
* aware of time zone but does support {@code DISTINCT}.
*/
@SuppressWarnings("overloads") // These are ambiguous if you aren't using ctor references but we always do
protected static <T extends Function> FunctionDefinition def(Class<T> function,
DistinctAwareUnaryFunctionBuilder<T> ctorRef, String... aliases) {
FunctionBuilder builder = (location, children, distinct, tz) -> {
if (children.size() != 1) {
throw new IllegalArgumentException("expects only a single argument");
}
return ctorRef.build(location, children.get(0), distinct);
};
return def(function, builder, aliases);
}
protected interface DistinctAwareUnaryFunctionBuilder<T> {
T build(Location location, Expression target, boolean distinct);
}
/**
* Build a {@linkplain FunctionDefinition} for a unary function that is
* aware of time zone and does not support {@code DISTINCT}.
*/
@SuppressWarnings("overloads") // These are ambiguous if you aren't using ctor references but we always do
protected static <T extends Function> FunctionDefinition def(Class<T> function,
TimeZoneAwareUnaryFunctionBuilder<T> ctorRef, String... aliases) {
FunctionBuilder builder = (location, children, distinct, tz) -> {
if (children.size() != 1) {
throw new IllegalArgumentException("expects only a single argument");
}
if (distinct) {
throw new IllegalArgumentException("does not support DISTINCT yet it was specified");
}
return ctorRef.build(location, children.get(0), tz);
};
return def(function, builder, aliases);
}
protected interface TimeZoneAwareUnaryFunctionBuilder<T> {
T build(Location location, Expression target, DateTimeZone tz);
}
/**
* Build a {@linkplain FunctionDefinition} for a binary function that is
* not aware of time zone and does not support {@code DISTINCT}.
*/
@SuppressWarnings("overloads") // These are ambiguous if you aren't using ctor references but we always do
protected static <T extends Function> FunctionDefinition def(Class<T> function,
BinaryFunctionBuilder<T> ctorRef, String... aliases) {
FunctionBuilder builder = (location, children, distinct, tz) -> {
if (children.size() != 2) {
throw new IllegalArgumentException("expects only a single argument");
}
if (distinct) {
throw new IllegalArgumentException("does not support DISTINCT yet it was specified");
}
return ctorRef.build(location, children.get(0), children.get(1));
};
return def(function, builder, aliases);
}
protected interface BinaryFunctionBuilder<T> {
T build(Location location, Expression lhs, Expression rhs);
}
private static FunctionDefinition def(Class<? extends Function> function, FunctionBuilder builder, String... aliases) {
String primaryName = normalize(function.getSimpleName());
BiFunction<UnresolvedFunction, DateTimeZone, Function> realBuilder = (uf, tz) -> {
try {
return builder.build(uf.location(), uf.children(), uf.distinct(), tz);
} catch (IllegalArgumentException e) {
throw new ParsingException("error builder [" + primaryName + "]: " + e.getMessage(), e,
uf.location().getLineNumber(), uf.location().getColumnNumber());
}
};
return new FunctionDefinition(primaryName, unmodifiableList(Arrays.asList(aliases)), function, realBuilder);
}
private interface FunctionBuilder {
Function build(Location location, List<Expression> children, boolean distinct, DateTimeZone tz);
}
protected static String normalize(String name) {
// translate CamelCase to camel_case
return StringUtils.camelCaseToUnderscore(name);
}
//
// Instantiates a function through reflection.
// Picks up the constructor by expecting to be of type (Location,Expression) or (Location,List<Expression>) depending on the size of given children, parameters.
// If the function has certain 'aware'-ness (based on the interface implemented), the appropriate types are added to the signature
@SuppressWarnings("rawtypes")
private static Function createInstance(Class<? extends Function> clazz, UnresolvedFunction ur, DateTimeZone timeZone) {
NodeInfo info = NodeUtils.info((Class<? extends Node>) clazz);
Class<?>[] pTypes = info.ctr.getParameterTypes();
boolean distinctAware = DistinctAware.class.isAssignableFrom(clazz);
boolean timezoneAware = TimeZoneAware.class.isAssignableFrom(clazz);
// constructor types - location - distinct? - timezone?
int expectedParamCount = pTypes.length - (1 + (distinctAware ? 1 : 0) + (timezoneAware ? 1 : 0));
// check constructor signature
if (ur.children().size() != expectedParamCount) {
List<String> expected = new ArrayList<>();
for (int i = 1; i < expectedParamCount; i++) {
expected.add(pTypes[i].getSimpleName());
}
throw new ParsingException(ur.location(), "Invalid number of arguments given to function [%s], expected %d argument(s):%s but received %d:%s",
ur.name(), expected.size(), expected.toString(), ur.children().size(), ur.children());
}
// validate distinct ctor
if (!distinctAware && ur.distinct()) {
throw new ParsingException(ur.location(), "Function [%s] does not support DISTINCT yet it was specified", ur.name());
}
// List<Class> ctorSignature = new ArrayList<>();
// ctorSignature.add(Location.class);
//
// // might be a constant function
// if (expVal instanceof List && ((List) expVal).isEmpty()) {
// noExpression = Arrays.equals(new Class[] { Location.class }, info.ctr.getParameterTypes());
// }
// else {
// ctorSignature.add(exp);
// }
//
// // aware stuff
// if (distinctAware) {
// ctorSignature.add(boolean.class);
// }
// if (timezoneAware) {
// ctorSignature.add(DateTimeZone.class);
// }
//
// // validate
// Assert.isTrue(Arrays.equals(ctorSignature.toArray(new Class[ctorSignature.size()]), info.ctr.getParameterTypes()),
// "No constructor with signature %s found for [%s], found %s instead", ctorSignature, clazz.getTypeName(), info.ctr);
// now add the actual values
try {
List<Object> args = new ArrayList<>();
// always add location first
args.add(ur.location());
// has multiple arguments
args.addAll(ur.children());
if (distinctAware) {
args.add(ur.distinct());
}
if (timezoneAware) {
args.add(timeZone);
}
return (Function) info.ctr.newInstance(args.toArray());
} catch (InstantiationException | IllegalAccessException | IllegalArgumentException | InvocationTargetException ex) {
throw new SqlIllegalArgumentException(ex, "Cannot create instance of function %s", ur.name());
}
}
}

View File

@ -6,7 +6,6 @@
package org.elasticsearch.xpack.sql.expression.function;
import org.elasticsearch.xpack.sql.expression.function.Score;
import org.elasticsearch.xpack.sql.expression.function.aggregate.AggregateFunction;
import org.elasticsearch.xpack.sql.expression.function.aggregate.Avg;
import org.elasticsearch.xpack.sql.expression.function.aggregate.Correlation;
import org.elasticsearch.xpack.sql.expression.function.aggregate.Count;
@ -25,7 +24,6 @@ import org.elasticsearch.xpack.sql.expression.function.aggregate.StddevPop;
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.ScalarFunction;
import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DayOfMonth;
import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DayOfWeek;
import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DayOfYear;
@ -57,98 +55,72 @@ import org.elasticsearch.xpack.sql.expression.function.scalar.math.Sin;
import org.elasticsearch.xpack.sql.expression.function.scalar.math.Sinh;
import org.elasticsearch.xpack.sql.expression.function.scalar.math.Sqrt;
import org.elasticsearch.xpack.sql.expression.function.scalar.math.Tan;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.TreeMap;
import static java.util.Collections.unmodifiableMap;
import static java.util.Collections.unmodifiableList;
public class DefaultFunctionRegistry extends AbstractFunctionRegistry {
private static final Collection<Class<? extends Function>> FUNCTIONS = unmodifiableList(Arrays.asList(
private static final List<FunctionDefinition> FUNCTIONS = unmodifiableList(Arrays.asList(
// Aggregate functions
Avg.class,
Count.class,
Max.class,
Min.class,
Sum.class,
def(Avg.class, Avg::new),
def(Count.class, Count::new),
def(Max.class, Max::new),
def(Min.class, Min::new),
def(Sum.class, Sum::new),
// Statistics
Mean.class,
StddevPop.class,
VarPop.class,
Percentile.class,
PercentileRank.class,
SumOfSquares.class,
def(Mean.class, Mean::new),
def(StddevPop.class, StddevPop::new),
def(VarPop.class, VarPop::new),
def(Percentile.class, Percentile::new),
def(PercentileRank.class, PercentileRank::new),
def(SumOfSquares.class, SumOfSquares::new),
// Matrix aggs
MatrixCount.class,
MatrixMean.class,
MatrixVariance.class,
Skewness.class,
Kurtosis.class,
Covariance.class,
Correlation.class,
def(MatrixCount.class, MatrixCount::new),
def(MatrixMean.class, MatrixMean::new),
def(MatrixVariance.class, MatrixVariance::new),
def(Skewness.class, Skewness::new),
def(Kurtosis.class, Kurtosis::new),
def(Covariance.class, Covariance::new),
def(Correlation.class, Correlation::new),
// Scalar functions
// Date
DayOfMonth.class,
DayOfWeek.class,
DayOfYear.class,
HourOfDay.class,
MinuteOfDay.class,
MinuteOfHour.class,
SecondOfMinute.class,
MonthOfYear.class,
Year.class,
def(DayOfMonth.class, DayOfMonth::new, "DAY", "DOM"),
def(DayOfWeek.class, DayOfWeek::new, "DOW"),
def(DayOfYear.class, DayOfYear::new, "DOY"),
def(HourOfDay.class, HourOfDay::new, "HOUR"),
def(MinuteOfDay.class, MinuteOfDay::new),
def(MinuteOfHour.class, MinuteOfHour::new, "MINUTE"),
def(SecondOfMinute.class, SecondOfMinute::new, "SECOND"),
def(MonthOfYear.class, MonthOfYear::new, "MONTH"),
def(Year.class, Year::new),
// Math
Abs.class,
ACos.class,
ASin.class,
ATan.class,
Cbrt.class,
Ceil.class,
Cos.class,
Cosh.class,
Degrees.class,
E.class,
Exp.class,
Expm1.class,
Floor.class,
Log.class,
Log10.class,
Pi.class,
Radians.class,
Round.class,
Sin.class,
Sinh.class,
Sqrt.class,
Tan.class,
def(Abs.class, Abs::new),
def(ACos.class, ACos::new),
def(ASin.class, ASin::new),
def(ATan.class, ATan::new),
def(Cbrt.class, Cbrt::new),
def(Ceil.class, Ceil::new),
def(Cos.class, Cos::new),
def(Cosh.class, Cosh::new),
def(Degrees.class, Degrees::new),
def(E.class, E::new),
def(Exp.class, Exp::new),
def(Expm1.class, Expm1::new),
def(Floor.class, Floor::new),
def(Log.class, Log::new),
def(Log10.class, Log10::new),
def(Pi.class, Pi::new),
def(Radians.class, Radians::new),
def(Round.class, Round::new),
def(Sin.class, Sin::new),
def(Sinh.class, Sinh::new),
def(Sqrt.class, Sqrt::new),
def(Tan.class, Tan::new),
// Special
Score.class));
def(Score.class, Score::new)));
private static final Map<String, String> ALIASES;
static {
Map<String, String> aliases = new TreeMap<>();
aliases.put("DAY", "DAY_OF_MONTH");
aliases.put("DOM", "DAY_OF_MONTH");
aliases.put("DOW", "DAY_OF_WEEK");
aliases.put("DOY", "DAY_OF_YEAR");
aliases.put("HOUR", "HOUR_OF_DAY");
aliases.put("MINUTE", "MINUTE_OF_HOUR");
aliases.put("MONTH", "MONTH_OF_YEAR");
aliases.put("SECOND", "SECOND_OF_MINUTE");
ALIASES = unmodifiableMap(aliases);
}
@Override
protected Collection<Class<? extends Function>> functions() {
return FUNCTIONS;
}
@Override
protected Map<String, String> aliases() {
return ALIASES;
public DefaultFunctionRegistry() {
super(FUNCTIONS);
}
}

View File

@ -8,6 +8,8 @@ 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;
@ -16,12 +18,15 @@ public class FunctionDefinition {
private final String name;
private final List<String> aliases;
private final Class<? extends Function> clazz;
private final BiFunction<UnresolvedFunction, DateTimeZone, Function> builder;
private final FunctionType type;
FunctionDefinition(String name, List<String> aliases, Class<? extends Function> clazz) {
FunctionDefinition(String name, List<String> aliases,
Class<? extends Function> clazz, BiFunction<UnresolvedFunction, DateTimeZone, Function> builder) {
this.name = name;
this.aliases = aliases;
this.clazz = clazz;
this.builder = builder;
this.type = FunctionType.of(clazz);
}
@ -41,6 +46,10 @@ public class FunctionDefinition {
return clazz;
}
BiFunction<UnresolvedFunction, DateTimeZone, Function> builder() {
return builder;
}
@Override
public int hashCode() {
return Objects.hash(clazz);

View File

@ -7,12 +7,11 @@ package org.elasticsearch.xpack.sql.expression.function.aggregate;
import org.elasticsearch.xpack.sql.expression.Expression;
import org.elasticsearch.xpack.sql.expression.NamedExpression;
import org.elasticsearch.xpack.sql.expression.function.aware.DistinctAware;
import org.elasticsearch.xpack.sql.tree.Location;
import org.elasticsearch.xpack.sql.type.DataType;
import org.elasticsearch.xpack.sql.type.DataTypes;
public class Count extends AggregateFunction implements DistinctAware {
public class Count extends AggregateFunction {
private final boolean distinct;

View File

@ -1,10 +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.aware;
public interface DistinctAware {
}

View File

@ -1,10 +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.aware;
public interface TimeZoneAware {
}

View File

@ -9,7 +9,6 @@ import org.elasticsearch.xpack.sql.expression.Expression;
import org.elasticsearch.xpack.sql.expression.Expressions;
import org.elasticsearch.xpack.sql.expression.FieldAttribute;
import org.elasticsearch.xpack.sql.expression.function.aggregate.AggregateFunctionAttribute;
import org.elasticsearch.xpack.sql.expression.function.aware.TimeZoneAware;
import org.elasticsearch.xpack.sql.expression.function.scalar.UnaryScalarFunction;
import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DateTimeProcessor.DateTimeExtractor;
import org.elasticsearch.xpack.sql.expression.function.scalar.processor.definition.ProcessorDefinition;
@ -27,7 +26,7 @@ import java.time.temporal.ChronoField;
import static org.elasticsearch.xpack.sql.expression.function.scalar.script.ParamsBuilder.paramsBuilder;
import static org.elasticsearch.xpack.sql.expression.function.scalar.script.ScriptTemplate.formatTemplate;
public abstract class DateTimeFunction extends UnaryScalarFunction implements TimeZoneAware {
public abstract class DateTimeFunction extends UnaryScalarFunction {
private final DateTimeZone timeZone;
private final String name;

View File

@ -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.function.aware.TimeZoneAware;
import org.elasticsearch.xpack.sql.tree.Location;
import org.joda.time.DateTimeZone;
@ -14,7 +13,7 @@ import org.joda.time.DateTimeZone;
* DateTimeFunctions that can be mapped as histogram. This means the dates order is maintained
* Unfortunately this means only YEAR works since everything else changes the order
*/
public abstract class DateTimeHistogramFunction extends DateTimeFunction implements TimeZoneAware {
public abstract class DateTimeHistogramFunction extends DateTimeFunction {
DateTimeHistogramFunction(Location location, Expression field, DateTimeZone timeZone) {
super(location, field, timeZone);

View File

@ -18,7 +18,7 @@ public class ParsingException extends ClientSqlException {
private final int line;
private final int charPositionInLine;
public ParsingException(String message, RecognitionException cause, int line, int charPositionInLine) {
public ParsingException(String message, Exception cause, int line, int charPositionInLine) {
super(message, cause);
this.line = line;