Skip to content

Commit

Permalink
QL: Refactor FunctionRegistry to make it pluggable (#67761)
Browse files Browse the repository at this point in the history
Break the FunctionRegistry monolith into a common QL base and a SQL
specific registry that handles aspects such as distinct and extract.
In the process clean-up the names and signature of internal interfaces.
Most of the semantics were preserved however the error messages were
slightly tweaked to make them more readable - this shouldn't be a
problem as they are being used internally mainly in test assertions.
  • Loading branch information
costin authored Jan 21, 2021
1 parent 6130945 commit 2f2b690
Show file tree
Hide file tree
Showing 9 changed files with 448 additions and 381 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@
public class EqlFunctionRegistry extends FunctionRegistry {

public EqlFunctionRegistry() {
super(functions());
register(functions());
}

private static FunctionDefinition[][] functions() {
private FunctionDefinition[][] functions() {
return new FunctionDefinition[][] {
// Scalar functions
// String
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,31 +14,26 @@
public class FunctionDefinition {
/**
* Converts an {@link UnresolvedFunction} into the a proper {@link Function}.
* <p>
* Provides the basic signature (unresolved function + runtime configuration object) while
* allowing extensions through the vararg extras which subclasses should expand for their
* own purposes.
*/
@FunctionalInterface
public interface Builder {
Function build(UnresolvedFunction uf, boolean distinct, Configuration configuration);
Function build(UnresolvedFunction uf, Configuration configuration, Object... extras);
}

private final String name;
private final List<String> aliases;
private final Class<? extends Function> clazz;
private final Builder builder;

/**
* Is this a datetime function compatible with {@code EXTRACT}.
*/
// TODO: needs refactoring so that specific function properties (per language) are isolated from QL
private final boolean extractViable;


protected FunctionDefinition(String name, List<String> aliases, Class<? extends Function> clazz, boolean dateTime, Builder builder) {
protected FunctionDefinition(String name, List<String> aliases, Class<? extends Function> clazz, Builder builder) {
this.name = name;
this.aliases = aliases;
this.clazz = clazz;
this.builder = builder;

this.extractViable = dateTime;
}

public String name() {
Expand All @@ -53,17 +48,10 @@ public Class<? extends Function> clazz() {
return clazz;
}

public Builder builder() {
protected Builder builder() {
return builder;
}

/**
* Is this a datetime function compatible with {@code EXTRACT}.
*/
public boolean extractViable() {
return extractViable;
}

@Override
public String toString() {
return format(null, "{}({})", name, aliases.isEmpty() ? "" : aliases.size() == 1 ? aliases.get(0) : aliases);
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public interface FunctionResolutionStrategy {
* Build the real function from this one and resolution metadata.
*/
default Function buildResolved(UnresolvedFunction uf, Configuration cfg, FunctionDefinition def) {
return def.builder().build(uf, false, cfg);
return def.builder().build(uf, cfg);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,19 +32,19 @@ public class FunctionRegistryTests extends ESTestCase {

public void testNoArgFunction() {
UnresolvedFunction ur = uf(DEFAULT);
FunctionRegistry r = new FunctionRegistry(def(DummyFunction.class, DummyFunction::new, "DUMMY_FUNCTION"));
FunctionRegistry r = new FunctionRegistry(defineDummyNoArgFunction());
FunctionDefinition def = r.resolveFunction(ur.name());
assertEquals(ur.source(), ur.buildResolved(randomConfiguration(), def).source());
}

public static FunctionDefinition defineDummyNoArgFunction() {
return def(DummyFunction.class, DummyFunction::new, "DUMMY_FUNCTION");
}

public void testUnaryFunction() {
UnresolvedFunction ur = uf(DEFAULT, mock(Expression.class));
FunctionRegistry r = new FunctionRegistry(def(DummyFunction.class, (Source l, Expression e) -> {
assertSame(e, ur.children().get(0));
return new DummyFunction(l);
}, "DUMMY_FUNCTION"));
FunctionRegistry r = new FunctionRegistry(defineDummyUnaryFunction(ur));
FunctionDefinition def = r.resolveFunction(ur.name());
assertFalse(def.extractViable());
assertEquals(ur.source(), ur.buildResolved(randomConfiguration(), def).source());

// No children aren't supported
Expand All @@ -58,6 +58,13 @@ public void testUnaryFunction() {
assertThat(e.getMessage(), endsWith("expects exactly one argument"));
}

public static FunctionDefinition defineDummyUnaryFunction(UnresolvedFunction ur) {
return def(DummyFunction.class, (Source l, Expression e) -> {
assertSame(e, ur.children().get(0));
return new DummyFunction(l);
}, "DUMMY_FUNCTION");
}

public void testBinaryFunction() {
UnresolvedFunction ur = uf(DEFAULT, mock(Expression.class), mock(Expression.class));
FunctionRegistry r = new FunctionRegistry(def(DummyFunction.class, (Source l, Expression lhs, Expression rhs) -> {
Expand All @@ -67,7 +74,6 @@ public void testBinaryFunction() {
}, "DUMMY_FUNCTION"));
FunctionDefinition def = r.resolveFunction(ur.name());
assertEquals(ur.source(), ur.buildResolved(randomConfiguration(), def).source());
assertFalse(def.extractViable());

// No children aren't supported
ParsingException e = expectThrows(ParsingException.class, () ->
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* 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;

import org.elasticsearch.xpack.ql.expression.function.Function;
import org.elasticsearch.xpack.ql.expression.function.FunctionDefinition;

import java.util.List;

public class SqlFunctionDefinition extends FunctionDefinition {

/**
* Is this a datetime function compatible with {@code EXTRACT}.
*/
private final boolean extractViable;

protected SqlFunctionDefinition(String name,
List<String> aliases,
Class<? extends Function> clazz,
boolean dateTime,
Builder builder) {
super(name, aliases, clazz, builder);
this.extractViable = dateTime;
}

/**
* Is this a datetime function compatible with {@code EXTRACT}.
*/
public boolean extractViable() {
return extractViable;
}

@Override
protected Builder builder() {
return super.builder();
}
}
Loading

0 comments on commit 2f2b690

Please sign in to comment.