yashmayya commented on code in PR #13573:
URL: https://github.com/apache/pinot/pull/13573#discussion_r1671753878
##########
pinot-spi/src/main/java/org/apache/pinot/spi/annotations/ScalarFunction.java:
##########
@@ -54,14 +54,13 @@
/**
* Whether the scalar function expects and can handle null arguments.
- *
*/
boolean nullableParameters() default false;
- boolean isPlaceholder() default false;
-
/**
* Whether the scalar function takes various number of arguments.
*/
boolean isVarArg() default false;
+
+ @Deprecated boolean isPlaceholder() default false;
Review Comment:
I didn't quite get what the purpose of this method was earlier? Also, any
particular reason for keeping it around as deprecated rather than removing it
(since there are no usages in this codebase currently)? Do we support
externally created scalar functions as plugins?
##########
pinot-common/src/main/java/org/apache/pinot/common/function/PinotScalarFunction.java:
##########
@@ -0,0 +1,57 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.function;
+
+import javax.annotation.Nullable;
+import org.apache.pinot.common.function.sql.PinotSqlFunction;
+import org.apache.pinot.common.utils.DataSchema.ColumnDataType;
+import org.apache.pinot.spi.annotations.ScalarFunction;
+
+
+/**
+ * Provides finer control to the scalar functions annotated with {@link
ScalarFunction}.
+ */
+public interface PinotScalarFunction {
+
+ /**
+ * Returns the name of the function.
+ */
+ String getName();
+
+ /**
+ * Returns the corresponding {@link PinotSqlFunction} to be registered into
the OperatorTable, or {@link null} if
Review Comment:
```suggestion
* Returns the corresponding {@link PinotSqlFunction} to be registered
into the OperatorTable, or {@code null} if
```
nit: `{@link null}` is invalid Javadoc (similarly elsewhere)
##########
pinot-common/src/main/java/org/apache/pinot/common/function/PinotScalarFunction.java:
##########
@@ -0,0 +1,57 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.function;
+
+import javax.annotation.Nullable;
+import org.apache.pinot.common.function.sql.PinotSqlFunction;
+import org.apache.pinot.common.utils.DataSchema.ColumnDataType;
+import org.apache.pinot.spi.annotations.ScalarFunction;
+
+
+/**
+ * Provides finer control to the scalar functions annotated with {@link
ScalarFunction}.
+ */
+public interface PinotScalarFunction {
+
+ /**
+ * Returns the name of the function.
+ */
+ String getName();
+
+ /**
+ * Returns the corresponding {@link PinotSqlFunction} to be registered into
the OperatorTable, or {@link null} if
+ * it doesn't need to be registered (e.g. standard SqlFunction).
+ */
+ @Nullable
+ PinotSqlFunction toPinotSqlFunction();
+
+ /**
+ * Returns the {@link FunctionInfo} for the given argument types, or {@link
null} if there is no matching.
+ */
+ @Nullable
+ default FunctionInfo getFunctionInfo(ColumnDataType[] argumentTypes) {
Review Comment:
Is the idea to implement this method in something like a
`ArgumentTypeBasedScalarFunction` in the future to support polymorphic scalar
functions?
##########
pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java:
##########
@@ -46,44 +56,92 @@ private FunctionRegistry() {
private static final Logger LOGGER =
LoggerFactory.getLogger(FunctionRegistry.class);
- // TODO: consolidate the following 2
- // This FUNCTION_INFO_MAP is used by Pinot server to look up function by #
of arguments
- private static final Map<String, Map<Integer, FunctionInfo>>
FUNCTION_INFO_MAP = new HashMap<>();
- // This FUNCTION_MAP is used by Calcite function catalog to look up function
by function signature.
- private static final NameMultimap<Function> FUNCTION_MAP = new
NameMultimap<>();
+ // Key is canonical name
+ public static final Map<String, PinotScalarFunction> FUNCTION_MAP;
private static final int VAR_ARG_KEY = -1;
/**
* Registers the scalar functions via reflection.
- * NOTE: In order to plugin methods using reflection, the methods should be
inside a class that includes ".function."
- * in its class path. This convention can significantly reduce the
time of class scanning.
+ * NOTE: In order to plugin functions or methods using reflection, they
should be inside a class that includes
Review Comment:
Is "functions" here referring to classes that are implementations of the
`PinotScalarFunction` interface?
##########
pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java:
##########
@@ -46,44 +56,92 @@ private FunctionRegistry() {
private static final Logger LOGGER =
LoggerFactory.getLogger(FunctionRegistry.class);
- // TODO: consolidate the following 2
- // This FUNCTION_INFO_MAP is used by Pinot server to look up function by #
of arguments
- private static final Map<String, Map<Integer, FunctionInfo>>
FUNCTION_INFO_MAP = new HashMap<>();
- // This FUNCTION_MAP is used by Calcite function catalog to look up function
by function signature.
- private static final NameMultimap<Function> FUNCTION_MAP = new
NameMultimap<>();
+ // Key is canonical name
+ public static final Map<String, PinotScalarFunction> FUNCTION_MAP;
private static final int VAR_ARG_KEY = -1;
/**
* Registers the scalar functions via reflection.
- * NOTE: In order to plugin methods using reflection, the methods should be
inside a class that includes ".function."
- * in its class path. This convention can significantly reduce the
time of class scanning.
+ * NOTE: In order to plugin functions or methods using reflection, they
should be inside a class that includes
+ * ".function." in its class path. This convention can significantly
reduce the time of class scanning.
*/
static {
long startTimeMs = System.currentTimeMillis();
+
+ // Register ScalarFunction classes
Review Comment:
Is this also being introduced here for future use with implementations of
the `PinotScalarFunction` interface to support polymorphic scalar functions?
##########
pinot-common/src/main/java/org/apache/pinot/common/function/TransformFunctionType.java:
##########
@@ -302,36 +263,22 @@ public enum TransformFunctionType {
private final String _name;
private final List<String> _alternativeNames;
- private final SqlKind _sqlKind;
private final SqlReturnTypeInference _returnTypeInference;
private final SqlOperandTypeChecker _operandTypeChecker;
- private final SqlFunctionCategory _sqlFunctionCategory;
TransformFunctionType(String name, String... alternativeNames) {
- this(name, null, null, null, null, alternativeNames);
- }
-
- TransformFunctionType(String name, SqlReturnTypeInference
returnTypeInference,
- SqlOperandTypeChecker operandTypeChecker, String... alternativeNames) {
- this(name, SqlKind.OTHER_FUNCTION, returnTypeInference, operandTypeChecker,
- SqlFunctionCategory.USER_DEFINED_FUNCTION, alternativeNames);
+ this(name, null, null, alternativeNames);
}
- /**
- * Constructor to use for transform functions which are supported in both v1
and multistage engines
- */
- TransformFunctionType(String name, SqlKind sqlKind, SqlReturnTypeInference
returnTypeInference,
- SqlOperandTypeChecker operandTypeChecker, SqlFunctionCategory
sqlFunctionCategory, String... alternativeNames) {
+ TransformFunctionType(String name, @Nullable SqlReturnTypeInference
returnTypeInference,
+ @Nullable SqlOperandTypeChecker operandTypeChecker, String...
alternativeNames) {
_name = name;
- List<String> all = new ArrayList<>(alternativeNames.length + 2);
- all.add(name);
- all.add(name());
- all.addAll(Arrays.asList(alternativeNames));
- _alternativeNames = Collections.unmodifiableList(all);
- _sqlKind = sqlKind;
+ List<String> names = new ArrayList<>(alternativeNames.length + 1);
+ names.add(name);
+ names.addAll(Arrays.asList(alternativeNames));
+ _alternativeNames = List.copyOf(names);
Review Comment:
nit: this member variable and its getter method should probably be renamed
considering that its not just the alternative names but all the names for the
function?
##########
pinot-core/src/test/java/org/apache/pinot/core/function/FunctionDefinitionRegistryTest.java:
##########
@@ -0,0 +1,106 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.function;
+
+import java.util.EnumSet;
+import org.apache.pinot.common.function.FunctionRegistry;
+import org.apache.pinot.common.function.TransformFunctionType;
+import org.apache.pinot.segment.spi.AggregationFunctionType;
+import org.apache.pinot.spi.annotations.ScalarFunction;
+import org.apache.pinot.sql.FilterKind;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertNull;
+import static org.testng.Assert.assertTrue;
+
+
+// NOTE: Keep this test in pinot-core to include all built-in scalar functions.
+// TODO: Consider breaking this test into multiple tests.
+public class FunctionDefinitionRegistryTest {
+ // TODO: Support these functions
+ private static final EnumSet<TransformFunctionType>
IGNORED_TRANSFORM_FUNCTION_TYPES = EnumSet.of(
+ // Special placeholder functions without implementation
+ TransformFunctionType.ARRAY_TO_MV,
TransformFunctionType.ARRAY_VALUE_CONSTRUCTOR, TransformFunctionType.SCALAR,
+ // Special functions that requires index
+ TransformFunctionType.JSON_EXTRACT_INDEX,
TransformFunctionType.MAP_VALUE, TransformFunctionType.LOOKUP,
+ // TODO: Support these functions
+ TransformFunctionType.IN, TransformFunctionType.NOT_IN,
TransformFunctionType.IS_TRUE,
+ TransformFunctionType.IS_NOT_TRUE, TransformFunctionType.IS_FALSE,
TransformFunctionType.IS_NOT_FALSE,
+ TransformFunctionType.AND, TransformFunctionType.OR,
TransformFunctionType.JSON_EXTRACT_SCALAR,
+ TransformFunctionType.JSON_EXTRACT_KEY,
TransformFunctionType.TIME_CONVERT,
+ TransformFunctionType.DATE_TIME_CONVERT_WINDOW_HOP,
TransformFunctionType.ARRAY_LENGTH,
+ TransformFunctionType.ARRAY_AVERAGE, TransformFunctionType.ARRAY_MIN,
TransformFunctionType.ARRAY_MAX,
+ TransformFunctionType.ARRAY_SUM, TransformFunctionType.VALUE_IN,
TransformFunctionType.IN_ID_SET,
+ TransformFunctionType.GROOVY, TransformFunctionType.CLP_DECODE,
TransformFunctionType.CLP_ENCODED_VARS_MATCH,
+ TransformFunctionType.ST_POLYGON, TransformFunctionType.ST_AREA);
+ private static final EnumSet<FilterKind> IGNORED_FILTER_KINDS = EnumSet.of(
+ // Special filter functions without implementation
+ FilterKind.TEXT_MATCH, FilterKind.TEXT_CONTAINS, FilterKind.JSON_MATCH,
FilterKind.VECTOR_SIMILARITY,
+ // TODO: Support these functions
+ FilterKind.AND, FilterKind.OR, FilterKind.RANGE, FilterKind.IN,
FilterKind.NOT_IN);
+
+ @Test
+ public void testCalciteFunctionMapAllRegistered() {
Review Comment:
Should this test's name be updated?
##########
pinot-core/src/test/java/org/apache/pinot/core/function/FunctionDefinitionRegistryTest.java:
##########
@@ -0,0 +1,106 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.function;
+
+import java.util.EnumSet;
+import org.apache.pinot.common.function.FunctionRegistry;
+import org.apache.pinot.common.function.TransformFunctionType;
+import org.apache.pinot.segment.spi.AggregationFunctionType;
+import org.apache.pinot.spi.annotations.ScalarFunction;
+import org.apache.pinot.sql.FilterKind;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertNull;
+import static org.testng.Assert.assertTrue;
+
+
+// NOTE: Keep this test in pinot-core to include all built-in scalar functions.
+// TODO: Consider breaking this test into multiple tests.
+public class FunctionDefinitionRegistryTest {
+ // TODO: Support these functions
+ private static final EnumSet<TransformFunctionType>
IGNORED_TRANSFORM_FUNCTION_TYPES = EnumSet.of(
+ // Special placeholder functions without implementation
+ TransformFunctionType.ARRAY_TO_MV,
TransformFunctionType.ARRAY_VALUE_CONSTRUCTOR, TransformFunctionType.SCALAR,
+ // Special functions that requires index
+ TransformFunctionType.JSON_EXTRACT_INDEX,
TransformFunctionType.MAP_VALUE, TransformFunctionType.LOOKUP,
+ // TODO: Support these functions
+ TransformFunctionType.IN, TransformFunctionType.NOT_IN,
TransformFunctionType.IS_TRUE,
+ TransformFunctionType.IS_NOT_TRUE, TransformFunctionType.IS_FALSE,
TransformFunctionType.IS_NOT_FALSE,
+ TransformFunctionType.AND, TransformFunctionType.OR,
TransformFunctionType.JSON_EXTRACT_SCALAR,
+ TransformFunctionType.JSON_EXTRACT_KEY,
TransformFunctionType.TIME_CONVERT,
+ TransformFunctionType.DATE_TIME_CONVERT_WINDOW_HOP,
TransformFunctionType.ARRAY_LENGTH,
+ TransformFunctionType.ARRAY_AVERAGE, TransformFunctionType.ARRAY_MIN,
TransformFunctionType.ARRAY_MAX,
+ TransformFunctionType.ARRAY_SUM, TransformFunctionType.VALUE_IN,
TransformFunctionType.IN_ID_SET,
+ TransformFunctionType.GROOVY, TransformFunctionType.CLP_DECODE,
TransformFunctionType.CLP_ENCODED_VARS_MATCH,
+ TransformFunctionType.ST_POLYGON, TransformFunctionType.ST_AREA);
Review Comment:
I don't get why some of these functions are considered as not supported?
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/TransformFunctionFactory.java:
##########
@@ -354,8 +354,8 @@ public static TransformFunction get(ExpressionContext
expression, Map<String, Co
public static TransformFunction get(ExpressionContext expression,
Map<String, DataSource> dataSourceMap) {
Map<String, ColumnContext> columnContextMap = new
HashMap<>(HashUtil.getHashMapCapacity(dataSourceMap.size()));
dataSourceMap.forEach((k, v) -> columnContextMap.put(k,
ColumnContext.fromDataSource(v)));
- QueryContext dummy = QueryContextConverterUtils.getQueryContext(
- CalciteSqlParser.compileToPinotQuery("SELECT * from testTable;"));
+ QueryContext dummy =
Review Comment:
Shouldn't this method ideally be in some test utils class rather than in
this class?
##########
pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java:
##########
@@ -95,108 +153,152 @@ public static void init() {
}
/**
- * Registers a method with the name of the method.
+ * Registers a {@link PinotScalarFunction} under the given canonical name.
*/
- public static void registerFunction(Method method, boolean
nullableParameters, boolean isPlaceholder,
- boolean isVarArg) {
- registerFunction(method.getName(), method, nullableParameters,
isPlaceholder, isVarArg);
+ private static void register(String canonicalName, PinotScalarFunction
function,
+ Map<String, PinotScalarFunction> functionMap) {
+ Preconditions.checkState(functionMap.put(canonicalName, function) == null,
"Function: %s is already registered",
+ canonicalName);
}
/**
- * Registers a method with the given function name.
+ * Registers a {@link FunctionInfo} under the given canonical name.
*/
- public static void registerFunction(String functionName, Method method,
boolean nullableParameters,
- boolean isPlaceholder, boolean isVarArg) {
- if (!isPlaceholder) {
- registerFunctionInfoMap(functionName, method, nullableParameters,
isVarArg);
- }
- registerCalciteNamedFunctionMap(functionName, method, nullableParameters,
isVarArg);
+ private static void register(String canonicalName, FunctionInfo
functionInfo, int numArguments,
+ Map<String, Map<Integer, FunctionInfo>> functionInfoMap) {
+ Preconditions.checkState(
+ functionInfoMap.computeIfAbsent(canonicalName, k -> new
HashMap<>()).put(numArguments, functionInfo) == null,
+ "Function: %s with %s arguments is already registered", canonicalName,
+ numArguments == VAR_ARG_KEY ? "variable" : numArguments);
}
- private static void registerFunctionInfoMap(String functionName, Method
method, boolean nullableParameters,
- boolean isVarArg) {
- FunctionInfo functionInfo = new FunctionInfo(method,
method.getDeclaringClass(), nullableParameters);
- String canonicalName = canonicalize(functionName);
- Map<Integer, FunctionInfo> functionInfoMap =
FUNCTION_INFO_MAP.computeIfAbsent(canonicalName, k -> new HashMap<>());
- if (isVarArg) {
- FunctionInfo existFunctionInfo = functionInfoMap.put(VAR_ARG_KEY,
functionInfo);
- Preconditions.checkState(existFunctionInfo == null ||
existFunctionInfo.getMethod() == functionInfo.getMethod(),
- "Function: %s with variable number of parameters is already
registered", functionName);
- } else {
- FunctionInfo existFunctionInfo =
functionInfoMap.put(method.getParameterCount(), functionInfo);
- Preconditions.checkState(existFunctionInfo == null ||
existFunctionInfo.getMethod() == functionInfo.getMethod(),
- "Function: %s with %s parameters is already registered",
functionName, method.getParameterCount());
- }
- }
-
- private static void registerCalciteNamedFunctionMap(String functionName,
Method method, boolean nullableParameters,
- boolean isVarArg) {
- if (method.getAnnotation(Deprecated.class) == null) {
- FUNCTION_MAP.put(functionName, ScalarFunctionImpl.create(method));
- }
- }
-
- public static Map<String, List<Function>> getRegisteredCalciteFunctionMap() {
- return FUNCTION_MAP.map();
+ /**
+ * Returns {@code true} if the given canonical name is registered, {@code
false} otherwise.
+ */
+ public static boolean contains(String canonicalName) {
+ return FUNCTION_MAP.containsKey(canonicalName);
}
- public static Set<String> getRegisteredCalciteFunctionNames() {
- return FUNCTION_MAP.map().keySet();
+ @Deprecated
+ public static boolean containsFunction(String name) {
+ return contains(canonicalize(name));
}
/**
- * Returns {@code true} if the given function name is registered, {@code
false} otherwise.
+ * Returns the {@link FunctionInfo} associated with the given canonical name
and argument types, or {@code null} if
+ * there is no matching method. This method should be called after the
FunctionRegistry is initialized and all methods
+ * are already registered.
*/
- public static boolean containsFunction(String functionName) {
- return FUNCTION_INFO_MAP.containsKey(canonicalize(functionName));
+ @Nullable
+ public static FunctionInfo lookupFunctionInfo(String canonicalName,
ColumnDataType[] argumentTypes) {
+ PinotScalarFunction function = FUNCTION_MAP.get(canonicalName);
+ return function != null ? function.getFunctionInfo(argumentTypes) : null;
}
/**
- * Returns the {@link FunctionInfo} associated with the given function name
and number of parameters, or {@code null}
+ * Returns the {@link FunctionInfo} associated with the given canonical name
and number of arguments, or {@code null}
* if there is no matching method. This method should be called after the
FunctionRegistry is initialized and all
* methods are already registered.
+ * TODO: Move all usages to {@link #lookupFunctionInfo(String,
ColumnDataType[])}.
*/
@Nullable
- public static FunctionInfo getFunctionInfo(String functionName, int
numParameters) {
- Map<Integer, FunctionInfo> functionInfoMap =
FUNCTION_INFO_MAP.get(canonicalize(functionName));
- if (functionInfoMap != null) {
- FunctionInfo functionInfo = functionInfoMap.get(numParameters);
- if (functionInfo != null) {
- return functionInfo;
- }
- return functionInfoMap.get(VAR_ARG_KEY);
- }
- return null;
+ public static FunctionInfo lookupFunctionInfo(String canonicalName, int
numArguments) {
+ PinotScalarFunction function = FUNCTION_MAP.get(canonicalName);
+ return function != null ? function.getFunctionInfo(numArguments) : null;
}
- private static String canonicalize(String functionName) {
- return StringUtils.remove(functionName, '_').toLowerCase();
+ @Deprecated
+ @Nullable
+ public static FunctionInfo getFunctionInfo(String name, int numArguments) {
+ return lookupFunctionInfo(canonicalize(name), numArguments);
Review Comment:
Why is this deprecated? Isn't it more convenient to canonicalize the name
inside the function registry rather than making sure that the name is canonical
at every call site?
##########
pinot-query-planner/src/main/java/org/apache/pinot/calcite/rel/rules/PinotAggregateReduceFunctionsRule.java:
##########
@@ -16,21 +16,27 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.pinot.calcite.sql.fun;
+package org.apache.pinot.calcite.rel.rules;
-import org.apache.calcite.sql.SqlCall;
-import org.apache.calcite.sql.SqlNode;
-import org.apache.calcite.sql.fun.SqlCoalesceFunction;
-import org.apache.calcite.sql.validate.SqlValidator;
+import org.apache.calcite.rel.core.AggregateCall;
+import org.apache.calcite.rel.rules.AggregateReduceFunctionsRule;
+import org.apache.calcite.sql.SqlKind;
/**
- * Pinot supports native COALESCE function, thus no need to create CASE WHEN
conversion.
+ * Pinot customized version of {@link AggregateReduceFunctionsRule} which only
reduce on SUM and AVG.
*/
-public class PinotSqlCoalesceFunction extends SqlCoalesceFunction {
+public class PinotAggregateReduceFunctionsRule extends
AggregateReduceFunctionsRule {
+ public static final PinotAggregateReduceFunctionsRule INSTANCE =
+ new PinotAggregateReduceFunctionsRule(Config.DEFAULT);
+
+ private PinotAggregateReduceFunctionsRule(Config config) {
+ super(config);
+ }
@Override
- public SqlNode rewriteCall(SqlValidator validator, SqlCall call) {
- return call;
+ public boolean canReduce(AggregateCall call) {
+ SqlKind kind = call.getAggregation().getKind();
+ return kind == SqlKind.SUM || kind == SqlKind.AVG;
Review Comment:
Why do we need this customized rule? Which of the original Calcite rule's
reductions don't work in Pinot?
##########
pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java:
##########
@@ -95,108 +153,152 @@ public static void init() {
}
/**
- * Registers a method with the name of the method.
+ * Registers a {@link PinotScalarFunction} under the given canonical name.
*/
- public static void registerFunction(Method method, boolean
nullableParameters, boolean isPlaceholder,
- boolean isVarArg) {
- registerFunction(method.getName(), method, nullableParameters,
isPlaceholder, isVarArg);
+ private static void register(String canonicalName, PinotScalarFunction
function,
+ Map<String, PinotScalarFunction> functionMap) {
+ Preconditions.checkState(functionMap.put(canonicalName, function) == null,
"Function: %s is already registered",
+ canonicalName);
}
/**
- * Registers a method with the given function name.
+ * Registers a {@link FunctionInfo} under the given canonical name.
*/
- public static void registerFunction(String functionName, Method method,
boolean nullableParameters,
- boolean isPlaceholder, boolean isVarArg) {
- if (!isPlaceholder) {
- registerFunctionInfoMap(functionName, method, nullableParameters,
isVarArg);
- }
- registerCalciteNamedFunctionMap(functionName, method, nullableParameters,
isVarArg);
+ private static void register(String canonicalName, FunctionInfo
functionInfo, int numArguments,
+ Map<String, Map<Integer, FunctionInfo>> functionInfoMap) {
+ Preconditions.checkState(
+ functionInfoMap.computeIfAbsent(canonicalName, k -> new
HashMap<>()).put(numArguments, functionInfo) == null,
+ "Function: %s with %s arguments is already registered", canonicalName,
+ numArguments == VAR_ARG_KEY ? "variable" : numArguments);
}
- private static void registerFunctionInfoMap(String functionName, Method
method, boolean nullableParameters,
- boolean isVarArg) {
- FunctionInfo functionInfo = new FunctionInfo(method,
method.getDeclaringClass(), nullableParameters);
- String canonicalName = canonicalize(functionName);
- Map<Integer, FunctionInfo> functionInfoMap =
FUNCTION_INFO_MAP.computeIfAbsent(canonicalName, k -> new HashMap<>());
- if (isVarArg) {
- FunctionInfo existFunctionInfo = functionInfoMap.put(VAR_ARG_KEY,
functionInfo);
- Preconditions.checkState(existFunctionInfo == null ||
existFunctionInfo.getMethod() == functionInfo.getMethod(),
- "Function: %s with variable number of parameters is already
registered", functionName);
- } else {
- FunctionInfo existFunctionInfo =
functionInfoMap.put(method.getParameterCount(), functionInfo);
- Preconditions.checkState(existFunctionInfo == null ||
existFunctionInfo.getMethod() == functionInfo.getMethod(),
- "Function: %s with %s parameters is already registered",
functionName, method.getParameterCount());
- }
- }
-
- private static void registerCalciteNamedFunctionMap(String functionName,
Method method, boolean nullableParameters,
- boolean isVarArg) {
- if (method.getAnnotation(Deprecated.class) == null) {
- FUNCTION_MAP.put(functionName, ScalarFunctionImpl.create(method));
- }
- }
-
- public static Map<String, List<Function>> getRegisteredCalciteFunctionMap() {
- return FUNCTION_MAP.map();
+ /**
+ * Returns {@code true} if the given canonical name is registered, {@code
false} otherwise.
+ */
+ public static boolean contains(String canonicalName) {
+ return FUNCTION_MAP.containsKey(canonicalName);
}
- public static Set<String> getRegisteredCalciteFunctionNames() {
- return FUNCTION_MAP.map().keySet();
+ @Deprecated
+ public static boolean containsFunction(String name) {
+ return contains(canonicalize(name));
}
/**
- * Returns {@code true} if the given function name is registered, {@code
false} otherwise.
+ * Returns the {@link FunctionInfo} associated with the given canonical name
and argument types, or {@code null} if
+ * there is no matching method. This method should be called after the
FunctionRegistry is initialized and all methods
+ * are already registered.
*/
- public static boolean containsFunction(String functionName) {
- return FUNCTION_INFO_MAP.containsKey(canonicalize(functionName));
+ @Nullable
+ public static FunctionInfo lookupFunctionInfo(String canonicalName,
ColumnDataType[] argumentTypes) {
+ PinotScalarFunction function = FUNCTION_MAP.get(canonicalName);
+ return function != null ? function.getFunctionInfo(argumentTypes) : null;
}
/**
- * Returns the {@link FunctionInfo} associated with the given function name
and number of parameters, or {@code null}
+ * Returns the {@link FunctionInfo} associated with the given canonical name
and number of arguments, or {@code null}
* if there is no matching method. This method should be called after the
FunctionRegistry is initialized and all
* methods are already registered.
+ * TODO: Move all usages to {@link #lookupFunctionInfo(String,
ColumnDataType[])}.
*/
@Nullable
- public static FunctionInfo getFunctionInfo(String functionName, int
numParameters) {
- Map<Integer, FunctionInfo> functionInfoMap =
FUNCTION_INFO_MAP.get(canonicalize(functionName));
- if (functionInfoMap != null) {
- FunctionInfo functionInfo = functionInfoMap.get(numParameters);
- if (functionInfo != null) {
- return functionInfo;
- }
- return functionInfoMap.get(VAR_ARG_KEY);
- }
- return null;
+ public static FunctionInfo lookupFunctionInfo(String canonicalName, int
numArguments) {
+ PinotScalarFunction function = FUNCTION_MAP.get(canonicalName);
+ return function != null ? function.getFunctionInfo(numArguments) : null;
}
- private static String canonicalize(String functionName) {
- return StringUtils.remove(functionName, '_').toLowerCase();
+ @Deprecated
+ @Nullable
+ public static FunctionInfo getFunctionInfo(String name, int numArguments) {
+ return lookupFunctionInfo(canonicalize(name), numArguments);
}
- /**
- * Placeholders for scalar function, they register and represents the
signature for transform and filter predicate
- * so that v2 engine can understand and plan them correctly.
- */
- private static class PlaceholderScalarFunctions {
+ public static String canonicalize(String name) {
+ return StringUtils.remove(name, '_').toLowerCase();
+ }
+
+ public static class ArgumentCountBasedScalarFunction implements
PinotScalarFunction {
+ private final String _name;
+ private final Map<Integer, FunctionInfo> _functionInfoMap;
+
+ private ArgumentCountBasedScalarFunction(String name, Map<Integer,
FunctionInfo> functionInfoMap) {
+ _name = name.toUpperCase();
Review Comment:
Why are the canonical names lower case while the name here is upper case?
##########
pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java:
##########
@@ -95,108 +153,152 @@ public static void init() {
}
/**
- * Registers a method with the name of the method.
+ * Registers a {@link PinotScalarFunction} under the given canonical name.
*/
- public static void registerFunction(Method method, boolean
nullableParameters, boolean isPlaceholder,
- boolean isVarArg) {
- registerFunction(method.getName(), method, nullableParameters,
isPlaceholder, isVarArg);
+ private static void register(String canonicalName, PinotScalarFunction
function,
+ Map<String, PinotScalarFunction> functionMap) {
+ Preconditions.checkState(functionMap.put(canonicalName, function) == null,
"Function: %s is already registered",
+ canonicalName);
}
/**
- * Registers a method with the given function name.
+ * Registers a {@link FunctionInfo} under the given canonical name.
*/
- public static void registerFunction(String functionName, Method method,
boolean nullableParameters,
- boolean isPlaceholder, boolean isVarArg) {
- if (!isPlaceholder) {
- registerFunctionInfoMap(functionName, method, nullableParameters,
isVarArg);
- }
- registerCalciteNamedFunctionMap(functionName, method, nullableParameters,
isVarArg);
+ private static void register(String canonicalName, FunctionInfo
functionInfo, int numArguments,
+ Map<String, Map<Integer, FunctionInfo>> functionInfoMap) {
+ Preconditions.checkState(
+ functionInfoMap.computeIfAbsent(canonicalName, k -> new
HashMap<>()).put(numArguments, functionInfo) == null,
+ "Function: %s with %s arguments is already registered", canonicalName,
+ numArguments == VAR_ARG_KEY ? "variable" : numArguments);
}
- private static void registerFunctionInfoMap(String functionName, Method
method, boolean nullableParameters,
- boolean isVarArg) {
- FunctionInfo functionInfo = new FunctionInfo(method,
method.getDeclaringClass(), nullableParameters);
- String canonicalName = canonicalize(functionName);
- Map<Integer, FunctionInfo> functionInfoMap =
FUNCTION_INFO_MAP.computeIfAbsent(canonicalName, k -> new HashMap<>());
- if (isVarArg) {
- FunctionInfo existFunctionInfo = functionInfoMap.put(VAR_ARG_KEY,
functionInfo);
- Preconditions.checkState(existFunctionInfo == null ||
existFunctionInfo.getMethod() == functionInfo.getMethod(),
- "Function: %s with variable number of parameters is already
registered", functionName);
- } else {
- FunctionInfo existFunctionInfo =
functionInfoMap.put(method.getParameterCount(), functionInfo);
- Preconditions.checkState(existFunctionInfo == null ||
existFunctionInfo.getMethod() == functionInfo.getMethod(),
- "Function: %s with %s parameters is already registered",
functionName, method.getParameterCount());
- }
- }
-
- private static void registerCalciteNamedFunctionMap(String functionName,
Method method, boolean nullableParameters,
- boolean isVarArg) {
- if (method.getAnnotation(Deprecated.class) == null) {
- FUNCTION_MAP.put(functionName, ScalarFunctionImpl.create(method));
- }
- }
-
- public static Map<String, List<Function>> getRegisteredCalciteFunctionMap() {
- return FUNCTION_MAP.map();
+ /**
+ * Returns {@code true} if the given canonical name is registered, {@code
false} otherwise.
+ */
+ public static boolean contains(String canonicalName) {
+ return FUNCTION_MAP.containsKey(canonicalName);
}
- public static Set<String> getRegisteredCalciteFunctionNames() {
- return FUNCTION_MAP.map().keySet();
+ @Deprecated
+ public static boolean containsFunction(String name) {
+ return contains(canonicalize(name));
}
/**
- * Returns {@code true} if the given function name is registered, {@code
false} otherwise.
+ * Returns the {@link FunctionInfo} associated with the given canonical name
and argument types, or {@code null} if
+ * there is no matching method. This method should be called after the
FunctionRegistry is initialized and all methods
+ * are already registered.
*/
- public static boolean containsFunction(String functionName) {
- return FUNCTION_INFO_MAP.containsKey(canonicalize(functionName));
+ @Nullable
+ public static FunctionInfo lookupFunctionInfo(String canonicalName,
ColumnDataType[] argumentTypes) {
+ PinotScalarFunction function = FUNCTION_MAP.get(canonicalName);
+ return function != null ? function.getFunctionInfo(argumentTypes) : null;
}
/**
- * Returns the {@link FunctionInfo} associated with the given function name
and number of parameters, or {@code null}
+ * Returns the {@link FunctionInfo} associated with the given canonical name
and number of arguments, or {@code null}
* if there is no matching method. This method should be called after the
FunctionRegistry is initialized and all
* methods are already registered.
+ * TODO: Move all usages to {@link #lookupFunctionInfo(String,
ColumnDataType[])}.
*/
@Nullable
- public static FunctionInfo getFunctionInfo(String functionName, int
numParameters) {
- Map<Integer, FunctionInfo> functionInfoMap =
FUNCTION_INFO_MAP.get(canonicalize(functionName));
- if (functionInfoMap != null) {
- FunctionInfo functionInfo = functionInfoMap.get(numParameters);
- if (functionInfo != null) {
- return functionInfo;
- }
- return functionInfoMap.get(VAR_ARG_KEY);
- }
- return null;
+ public static FunctionInfo lookupFunctionInfo(String canonicalName, int
numArguments) {
+ PinotScalarFunction function = FUNCTION_MAP.get(canonicalName);
+ return function != null ? function.getFunctionInfo(numArguments) : null;
}
- private static String canonicalize(String functionName) {
- return StringUtils.remove(functionName, '_').toLowerCase();
+ @Deprecated
+ @Nullable
+ public static FunctionInfo getFunctionInfo(String name, int numArguments) {
+ return lookupFunctionInfo(canonicalize(name), numArguments);
}
- /**
- * Placeholders for scalar function, they register and represents the
signature for transform and filter predicate
- * so that v2 engine can understand and plan them correctly.
- */
- private static class PlaceholderScalarFunctions {
+ public static String canonicalize(String name) {
+ return StringUtils.remove(name, '_').toLowerCase();
+ }
+
+ public static class ArgumentCountBasedScalarFunction implements
PinotScalarFunction {
+ private final String _name;
+ private final Map<Integer, FunctionInfo> _functionInfoMap;
+
+ private ArgumentCountBasedScalarFunction(String name, Map<Integer,
FunctionInfo> functionInfoMap) {
+ _name = name.toUpperCase();
+ _functionInfoMap = functionInfoMap;
+ }
- @ScalarFunction(names = {"textContains", "text_contains"}, isPlaceholder =
true)
- public static boolean textContains(String text, String pattern) {
- throw new UnsupportedOperationException("Placeholder scalar function,
should not reach here");
+ @Override
+ public String getName() {
+ return _name;
}
- @ScalarFunction(names = {"textMatch", "text_match"}, isPlaceholder = true)
- public static boolean textMatch(String text, String pattern) {
- throw new UnsupportedOperationException("Placeholder scalar function,
should not reach here");
+ @Override
+ public PinotSqlFunction toPinotSqlFunction() {
+ return new PinotSqlFunction(_name, getReturnTypeInference(),
getOperandTypeChecker());
}
- @ScalarFunction(names = {"jsonMatch", "json_match"}, isPlaceholder = true)
- public static boolean jsonMatch(String text, String pattern) {
- throw new UnsupportedOperationException("Placeholder scalar function,
should not reach here");
+ private SqlReturnTypeInference getReturnTypeInference() {
+ return opBinding -> {
+ int numArguments = opBinding.getOperandCount();
+ FunctionInfo functionInfo = getFunctionInfo(numArguments);
+ Preconditions.checkState(functionInfo != null, "Failed to find
function: %s with %s arguments", _name,
+ numArguments);
+ Method method = functionInfo.getMethod();
+ Class<?> returnClass = method.getReturnType();
+ RelDataTypeFactory typeFactory = opBinding.getTypeFactory();
+ RelDataType returnType = returnClass == Object.class ?
typeFactory.createSqlType(SqlTypeName.ANY)
+ : JavaTypeFactoryImpl.toSql(typeFactory,
typeFactory.createJavaType(returnClass));
+
+ if (!functionInfo.hasNullableParameters()) {
+ // When any parameter is null, return is null
+ for (RelDataType type : opBinding.collectOperandTypes()) {
+ if (type.isNullable()) {
+ return typeFactory.createTypeWithNullability(returnType, true);
+ }
+ }
+ }
+
+ return method.isAnnotationPresent(Nullable.class) ?
typeFactory.createTypeWithNullability(returnType, true)
+ : returnType;
+ };
+ }
+
+ private SqlOperandTypeChecker getOperandTypeChecker() {
+ if (_functionInfoMap.containsKey(VAR_ARG_KEY)) {
+ return OperandTypes.VARIADIC;
+ }
+ if (_functionInfoMap.size() == 1) {
+ return
getOperandTypeChecker(_functionInfoMap.values().iterator().next().getMethod());
+ }
+ List<SqlOperandTypeChecker> operandTypeCheckers = new
ArrayList<>(_functionInfoMap.size());
+ for (FunctionInfo functionInfo : _functionInfoMap.values()) {
+
operandTypeCheckers.add(getOperandTypeChecker(functionInfo.getMethod()));
+ }
+ return OperandTypes.or(operandTypeCheckers.toArray(new
SqlOperandTypeChecker[0]));
+ }
+
+ private static SqlTypeFamily getSqlTypeFamily(Class<?> clazz) {
+ // NOTE: Pinot allows some non-standard type conversions such as
Timestamp <-> long, boolean <-> int etc. Do not
+ // restrict the type family for now. We only restrict the type
family for String so that cast can be added.
+ // Explicit cast is required to correctly convert boolean and
Timestamp to String.
+ // TODO: Revisit this.
+ return clazz == String.class ? SqlTypeFamily.CHARACTER :
SqlTypeFamily.ANY;
Review Comment:
I didn't quite follow why only `String` type is being handled as a special
case here, could you please elaborate? Is the idea that either implicit casting
or runtime function errors will handle other cases rather than imposing an
overly restrictive type checker at compile time?
##########
pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java:
##########
@@ -95,108 +153,152 @@ public static void init() {
}
/**
- * Registers a method with the name of the method.
+ * Registers a {@link PinotScalarFunction} under the given canonical name.
*/
- public static void registerFunction(Method method, boolean
nullableParameters, boolean isPlaceholder,
- boolean isVarArg) {
- registerFunction(method.getName(), method, nullableParameters,
isPlaceholder, isVarArg);
+ private static void register(String canonicalName, PinotScalarFunction
function,
+ Map<String, PinotScalarFunction> functionMap) {
+ Preconditions.checkState(functionMap.put(canonicalName, function) == null,
"Function: %s is already registered",
+ canonicalName);
}
/**
- * Registers a method with the given function name.
+ * Registers a {@link FunctionInfo} under the given canonical name.
*/
- public static void registerFunction(String functionName, Method method,
boolean nullableParameters,
- boolean isPlaceholder, boolean isVarArg) {
- if (!isPlaceholder) {
- registerFunctionInfoMap(functionName, method, nullableParameters,
isVarArg);
- }
- registerCalciteNamedFunctionMap(functionName, method, nullableParameters,
isVarArg);
+ private static void register(String canonicalName, FunctionInfo
functionInfo, int numArguments,
+ Map<String, Map<Integer, FunctionInfo>> functionInfoMap) {
+ Preconditions.checkState(
+ functionInfoMap.computeIfAbsent(canonicalName, k -> new
HashMap<>()).put(numArguments, functionInfo) == null,
+ "Function: %s with %s arguments is already registered", canonicalName,
+ numArguments == VAR_ARG_KEY ? "variable" : numArguments);
}
- private static void registerFunctionInfoMap(String functionName, Method
method, boolean nullableParameters,
- boolean isVarArg) {
- FunctionInfo functionInfo = new FunctionInfo(method,
method.getDeclaringClass(), nullableParameters);
- String canonicalName = canonicalize(functionName);
- Map<Integer, FunctionInfo> functionInfoMap =
FUNCTION_INFO_MAP.computeIfAbsent(canonicalName, k -> new HashMap<>());
- if (isVarArg) {
- FunctionInfo existFunctionInfo = functionInfoMap.put(VAR_ARG_KEY,
functionInfo);
- Preconditions.checkState(existFunctionInfo == null ||
existFunctionInfo.getMethod() == functionInfo.getMethod(),
- "Function: %s with variable number of parameters is already
registered", functionName);
- } else {
- FunctionInfo existFunctionInfo =
functionInfoMap.put(method.getParameterCount(), functionInfo);
- Preconditions.checkState(existFunctionInfo == null ||
existFunctionInfo.getMethod() == functionInfo.getMethod(),
- "Function: %s with %s parameters is already registered",
functionName, method.getParameterCount());
- }
- }
-
- private static void registerCalciteNamedFunctionMap(String functionName,
Method method, boolean nullableParameters,
- boolean isVarArg) {
- if (method.getAnnotation(Deprecated.class) == null) {
- FUNCTION_MAP.put(functionName, ScalarFunctionImpl.create(method));
- }
- }
-
- public static Map<String, List<Function>> getRegisteredCalciteFunctionMap() {
- return FUNCTION_MAP.map();
+ /**
+ * Returns {@code true} if the given canonical name is registered, {@code
false} otherwise.
+ */
+ public static boolean contains(String canonicalName) {
+ return FUNCTION_MAP.containsKey(canonicalName);
}
- public static Set<String> getRegisteredCalciteFunctionNames() {
- return FUNCTION_MAP.map().keySet();
+ @Deprecated
+ public static boolean containsFunction(String name) {
+ return contains(canonicalize(name));
}
/**
- * Returns {@code true} if the given function name is registered, {@code
false} otherwise.
+ * Returns the {@link FunctionInfo} associated with the given canonical name
and argument types, or {@code null} if
+ * there is no matching method. This method should be called after the
FunctionRegistry is initialized and all methods
+ * are already registered.
*/
- public static boolean containsFunction(String functionName) {
- return FUNCTION_INFO_MAP.containsKey(canonicalize(functionName));
+ @Nullable
+ public static FunctionInfo lookupFunctionInfo(String canonicalName,
ColumnDataType[] argumentTypes) {
+ PinotScalarFunction function = FUNCTION_MAP.get(canonicalName);
+ return function != null ? function.getFunctionInfo(argumentTypes) : null;
}
/**
- * Returns the {@link FunctionInfo} associated with the given function name
and number of parameters, or {@code null}
+ * Returns the {@link FunctionInfo} associated with the given canonical name
and number of arguments, or {@code null}
* if there is no matching method. This method should be called after the
FunctionRegistry is initialized and all
* methods are already registered.
+ * TODO: Move all usages to {@link #lookupFunctionInfo(String,
ColumnDataType[])}.
*/
@Nullable
- public static FunctionInfo getFunctionInfo(String functionName, int
numParameters) {
- Map<Integer, FunctionInfo> functionInfoMap =
FUNCTION_INFO_MAP.get(canonicalize(functionName));
- if (functionInfoMap != null) {
- FunctionInfo functionInfo = functionInfoMap.get(numParameters);
- if (functionInfo != null) {
- return functionInfo;
- }
- return functionInfoMap.get(VAR_ARG_KEY);
- }
- return null;
+ public static FunctionInfo lookupFunctionInfo(String canonicalName, int
numArguments) {
+ PinotScalarFunction function = FUNCTION_MAP.get(canonicalName);
+ return function != null ? function.getFunctionInfo(numArguments) : null;
}
- private static String canonicalize(String functionName) {
- return StringUtils.remove(functionName, '_').toLowerCase();
+ @Deprecated
+ @Nullable
+ public static FunctionInfo getFunctionInfo(String name, int numArguments) {
+ return lookupFunctionInfo(canonicalize(name), numArguments);
}
- /**
- * Placeholders for scalar function, they register and represents the
signature for transform and filter predicate
- * so that v2 engine can understand and plan them correctly.
- */
- private static class PlaceholderScalarFunctions {
+ public static String canonicalize(String name) {
+ return StringUtils.remove(name, '_').toLowerCase();
+ }
+
+ public static class ArgumentCountBasedScalarFunction implements
PinotScalarFunction {
+ private final String _name;
+ private final Map<Integer, FunctionInfo> _functionInfoMap;
+
+ private ArgumentCountBasedScalarFunction(String name, Map<Integer,
FunctionInfo> functionInfoMap) {
+ _name = name.toUpperCase();
+ _functionInfoMap = functionInfoMap;
+ }
- @ScalarFunction(names = {"textContains", "text_contains"}, isPlaceholder =
true)
- public static boolean textContains(String text, String pattern) {
- throw new UnsupportedOperationException("Placeholder scalar function,
should not reach here");
+ @Override
+ public String getName() {
+ return _name;
}
- @ScalarFunction(names = {"textMatch", "text_match"}, isPlaceholder = true)
- public static boolean textMatch(String text, String pattern) {
- throw new UnsupportedOperationException("Placeholder scalar function,
should not reach here");
+ @Override
+ public PinotSqlFunction toPinotSqlFunction() {
+ return new PinotSqlFunction(_name, getReturnTypeInference(),
getOperandTypeChecker());
}
- @ScalarFunction(names = {"jsonMatch", "json_match"}, isPlaceholder = true)
- public static boolean jsonMatch(String text, String pattern) {
- throw new UnsupportedOperationException("Placeholder scalar function,
should not reach here");
+ private SqlReturnTypeInference getReturnTypeInference() {
+ return opBinding -> {
+ int numArguments = opBinding.getOperandCount();
+ FunctionInfo functionInfo = getFunctionInfo(numArguments);
+ Preconditions.checkState(functionInfo != null, "Failed to find
function: %s with %s arguments", _name,
+ numArguments);
+ Method method = functionInfo.getMethod();
+ Class<?> returnClass = method.getReturnType();
+ RelDataTypeFactory typeFactory = opBinding.getTypeFactory();
+ RelDataType returnType = returnClass == Object.class ?
typeFactory.createSqlType(SqlTypeName.ANY)
+ : JavaTypeFactoryImpl.toSql(typeFactory,
typeFactory.createJavaType(returnClass));
+
+ if (!functionInfo.hasNullableParameters()) {
+ // When any parameter is null, return is null
+ for (RelDataType type : opBinding.collectOperandTypes()) {
+ if (type.isNullable()) {
+ return typeFactory.createTypeWithNullability(returnType, true);
+ }
+ }
+ }
+
+ return method.isAnnotationPresent(Nullable.class) ?
typeFactory.createTypeWithNullability(returnType, true)
+ : returnType;
+ };
+ }
+
+ private SqlOperandTypeChecker getOperandTypeChecker() {
+ if (_functionInfoMap.containsKey(VAR_ARG_KEY)) {
+ return OperandTypes.VARIADIC;
+ }
+ if (_functionInfoMap.size() == 1) {
+ return
getOperandTypeChecker(_functionInfoMap.values().iterator().next().getMethod());
+ }
+ List<SqlOperandTypeChecker> operandTypeCheckers = new
ArrayList<>(_functionInfoMap.size());
+ for (FunctionInfo functionInfo : _functionInfoMap.values()) {
+
operandTypeCheckers.add(getOperandTypeChecker(functionInfo.getMethod()));
+ }
+ return OperandTypes.or(operandTypeCheckers.toArray(new
SqlOperandTypeChecker[0]));
+ }
+
+ private static SqlTypeFamily getSqlTypeFamily(Class<?> clazz) {
+ // NOTE: Pinot allows some non-standard type conversions such as
Timestamp <-> long, boolean <-> int etc. Do not
+ // restrict the type family for now. We only restrict the type
family for String so that cast can be added.
+ // Explicit cast is required to correctly convert boolean and
Timestamp to String.
+ // TODO: Revisit this.
+ return clazz == String.class ? SqlTypeFamily.CHARACTER :
SqlTypeFamily.ANY;
+ }
+
+ private static SqlOperandTypeChecker getOperandTypeChecker(Method method) {
+ Class<?>[] parameterTypes = method.getParameterTypes();
+ int length = parameterTypes.length;
+ SqlTypeFamily[] typeFamilies = new SqlTypeFamily[length];
+ for (int i = 0; i < length; i++) {
+ typeFamilies[i] = getSqlTypeFamily(parameterTypes[i]);
+ }
+ return OperandTypes.family(typeFamilies);
}
- @ScalarFunction(names = {"vectorSimilarity", "vector_similarity"},
isPlaceholder = true)
- public static boolean vectorSimilarity(float[] vector1, float[] vector2,
int topk) {
- throw new UnsupportedOperationException("Placeholder scalar function,
should not reach here");
+ @Nullable
+ @Override
+ public FunctionInfo getFunctionInfo(int numArguments) {
+ FunctionInfo functionInfo = _functionInfoMap.get(numArguments);
+ return functionInfo != null ? functionInfo :
_functionInfoMap.get(VAR_ARG_KEY);
Review Comment:
This means that any scalar function that is annotated as vararg must support
any number of arguments including 0 - i.e., there's no concept of a minimum
number of args here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
