walterddr commented on code in PR #12302:
URL: https://github.com/apache/pinot/pull/12302#discussion_r1491512462
##########
pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java:
##########
@@ -94,83 +155,196 @@ private FunctionRegistry() {
public static void init() {
}
+ @VisibleForTesting
+ public static void registerFunction(Method method) {
+ registerFunction(method, Collections.singleton(method.getName()), false,
false);
+ }
+
+ @VisibleForTesting
+ public static Set<String> getRegisteredCalciteFunctionNames() {
+ return getFunctionMap().map().keySet();
+ }
+
+ /**
+ * Returns {@code true} if the given function name is registered, {@code
false} otherwise.
+ */
+ public static boolean containsFunction(String functionName) {
+ return getFunctionMap().containsKey(functionName, CASE_SENSITIVITY);
+ }
+
/**
- * Registers a method with the name of the method.
+ * Returns the {@link FunctionInfo} associated with the given function name
and number of parameters, 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 void registerFunction(Method method, boolean
nullableParameters, boolean isPlaceholder,
- boolean isVarArg) {
- registerFunction(method.getName(), method, nullableParameters,
isPlaceholder, isVarArg);
+ @Nullable
+ public static FunctionInfo getFunctionInfo(String functionName, int
numParams) {
+ return getFunctionInfoFromCalciteNamedMap(functionName, numParams);
}
/**
- * Registers a method with the given function name.
+ * Returns the {@link FunctionInfo} associated with the given function name
and number of parameters, 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 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 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);
+ @Nullable
+ public static FunctionInfo getFunctionInfo(SqlOperatorTable operatorTable,
RelDataTypeFactory typeFactory,
+ String functionName, List<RelDataType> argTypes) {
+ PinotScalarFunction scalarFunction = getScalarFunction(operatorTable,
typeFactory, functionName, argTypes);
+ if (scalarFunction != null) {
+ return scalarFunction.getFunctionInfo();
} 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());
+ // TODO: convert this to throw IAE when all operator has scalar
equivalent.
+ return null;
}
}
- 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));
+ @Nullable
+ private static FunctionInfo getFunctionInfoFromCalciteNamedMap(String
functionName, int numParams) {
+ List<PinotScalarFunction> candidates =
getFunctionMap().range(functionName, CASE_SENSITIVITY).stream().filter(
+ e -> e.getValue() instanceof PinotScalarFunction &&
(e.getValue().getParameters().size() == numParams
+ || ((PinotScalarFunction) e.getValue()).isVarArgs())).map(e ->
(PinotScalarFunction) e.getValue())
+ .collect(Collectors.toList());
+ if (candidates.size() == 1) {
+ return candidates.get(0).getFunctionInfo();
+ } else {
+ // TODO: convert this to throw IAE when all operator has scalar
equivalent.
+ return null;
}
}
- public static Map<String, List<Function>> getRegisteredCalciteFunctionMap() {
- return FUNCTION_MAP.map();
+ @Nullable
+ private static PinotScalarFunction getScalarFunction(SqlOperatorTable
operatorTable, RelDataTypeFactory typeFactory,
+ String functionName, List<RelDataType> argTypes) {
+ SqlOperator sqlOperator =
+ SqlUtil.lookupRoutine(operatorTable, typeFactory, new
SqlIdentifier(functionName, SqlParserPos.QUOTED_ZERO),
+ argTypes, null, null, SqlSyntax.FUNCTION, SqlKind.OTHER_FUNCTION,
+ SqlNameMatchers.withCaseSensitive(false), true);
+ if (sqlOperator instanceof SqlUserDefinedFunction) {
+ Function function = ((SqlUserDefinedFunction) sqlOperator).getFunction();
+ if (function instanceof PinotScalarFunction) {
+ return (PinotScalarFunction) function;
+ }
+ } else if (sqlOperator instanceof PinotSqlScalarFunction) {
+ return ((PinotSqlScalarFunction) sqlOperator).getFunction();
+ }
+ return null;
}
- public static Set<String> getRegisteredCalciteFunctionNames() {
- return FUNCTION_MAP.map().keySet();
+ public static NameMultimap<PinotFunction> getFunctionMap() {
+ return FUNCTION_MAP;
}
- /**
- * Returns {@code true} if the given function name is registered, {@code
false} otherwise.
- */
- public static boolean containsFunction(String functionName) {
- return FUNCTION_INFO_MAP.containsKey(canonicalize(functionName));
+ public static NameMultimap<SqlOperator> getOperatorMap() {
+ return OPERATOR_MAP;
}
- /**
- * Returns the {@link FunctionInfo} associated with the given function name
and number of parameters, 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.
- */
- @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;
+ private static void registerFunction(Method method, Set<String> alias,
boolean nullableParameters, boolean varArg) {
+ if (method.getAnnotation(Deprecated.class) == null) {
+ for (String name : alias) {
+ registerCalciteNamedFunctionMap(name, method, nullableParameters,
varArg);
}
- return functionInfoMap.get(VAR_ARG_KEY);
}
- return null;
}
- private static String canonicalize(String functionName) {
- return StringUtils.remove(functionName, '_').toLowerCase();
+ private static void registerFunction(Class<?> clazz, Set<String> alias,
boolean nullableParameters, boolean varArg) {
+ if (clazz.getAnnotation(Deprecated.class) == null) {
+ for (String name : alias) {
+ registerCalciteNamedFunctionMap(name, clazz, nullableParameters,
varArg);
+ }
+ }
+ }
+
+ private static void registerCalciteNamedFunctionMap(String name, Method
method, boolean nullableParameters,
+ boolean varArg) {
+ FUNCTION_MAP.put(name, new PinotScalarFunction(name, method,
nullableParameters, varArg));
+ }
+
+ private static void registerCalciteNamedFunctionMap(String name, Class<?>
clazz, boolean nullableParameters,
+ boolean varArg) {
+ try {
+ SqlReturnTypeInference returnTypeInference =
+ (SqlReturnTypeInference)
clazz.getField("RETURN_TYPE_INFERENCE").get(null);
+ SqlOperandTypeChecker operandTypeChecker =
+ (SqlOperandTypeChecker)
clazz.getField("OPERAND_TYPE_CHECKER").get(null);
+ for (Method method : clazz.getMethods()) {
+ if (method.getName().equals("eval")) {
Review Comment:
both are great suggestions/improvements. there's no reason, i simply fell
1. there's no need to annotate additionally to all callable methods if we
already have the class annotated (might be verbose with lots of polymorphism)
2. for the special fields yeah we can use interface method stub. it is just
didn't occur to me that we need different ones within the same annotated
function class
##########
pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java:
##########
@@ -94,83 +155,196 @@ private FunctionRegistry() {
public static void init() {
}
+ @VisibleForTesting
+ public static void registerFunction(Method method) {
+ registerFunction(method, Collections.singleton(method.getName()), false,
false);
+ }
+
+ @VisibleForTesting
+ public static Set<String> getRegisteredCalciteFunctionNames() {
+ return getFunctionMap().map().keySet();
+ }
+
+ /**
+ * Returns {@code true} if the given function name is registered, {@code
false} otherwise.
+ */
+ public static boolean containsFunction(String functionName) {
+ return getFunctionMap().containsKey(functionName, CASE_SENSITIVITY);
+ }
+
/**
- * Registers a method with the name of the method.
+ * Returns the {@link FunctionInfo} associated with the given function name
and number of parameters, 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 void registerFunction(Method method, boolean
nullableParameters, boolean isPlaceholder,
- boolean isVarArg) {
- registerFunction(method.getName(), method, nullableParameters,
isPlaceholder, isVarArg);
+ @Nullable
+ public static FunctionInfo getFunctionInfo(String functionName, int
numParams) {
+ return getFunctionInfoFromCalciteNamedMap(functionName, numParams);
}
/**
- * Registers a method with the given function name.
+ * Returns the {@link FunctionInfo} associated with the given function name
and number of parameters, 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 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 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);
+ @Nullable
+ public static FunctionInfo getFunctionInfo(SqlOperatorTable operatorTable,
RelDataTypeFactory typeFactory,
+ String functionName, List<RelDataType> argTypes) {
+ PinotScalarFunction scalarFunction = getScalarFunction(operatorTable,
typeFactory, functionName, argTypes);
+ if (scalarFunction != null) {
+ return scalarFunction.getFunctionInfo();
} 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());
+ // TODO: convert this to throw IAE when all operator has scalar
equivalent.
+ return null;
}
}
- 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));
+ @Nullable
+ private static FunctionInfo getFunctionInfoFromCalciteNamedMap(String
functionName, int numParams) {
+ List<PinotScalarFunction> candidates =
getFunctionMap().range(functionName, CASE_SENSITIVITY).stream().filter(
+ e -> e.getValue() instanceof PinotScalarFunction &&
(e.getValue().getParameters().size() == numParams
+ || ((PinotScalarFunction) e.getValue()).isVarArgs())).map(e ->
(PinotScalarFunction) e.getValue())
+ .collect(Collectors.toList());
+ if (candidates.size() == 1) {
+ return candidates.get(0).getFunctionInfo();
+ } else {
+ // TODO: convert this to throw IAE when all operator has scalar
equivalent.
+ return null;
}
}
- public static Map<String, List<Function>> getRegisteredCalciteFunctionMap() {
- return FUNCTION_MAP.map();
+ @Nullable
+ private static PinotScalarFunction getScalarFunction(SqlOperatorTable
operatorTable, RelDataTypeFactory typeFactory,
+ String functionName, List<RelDataType> argTypes) {
+ SqlOperator sqlOperator =
+ SqlUtil.lookupRoutine(operatorTable, typeFactory, new
SqlIdentifier(functionName, SqlParserPos.QUOTED_ZERO),
+ argTypes, null, null, SqlSyntax.FUNCTION, SqlKind.OTHER_FUNCTION,
+ SqlNameMatchers.withCaseSensitive(false), true);
+ if (sqlOperator instanceof SqlUserDefinedFunction) {
+ Function function = ((SqlUserDefinedFunction) sqlOperator).getFunction();
+ if (function instanceof PinotScalarFunction) {
+ return (PinotScalarFunction) function;
+ }
+ } else if (sqlOperator instanceof PinotSqlScalarFunction) {
+ return ((PinotSqlScalarFunction) sqlOperator).getFunction();
+ }
+ return null;
}
- public static Set<String> getRegisteredCalciteFunctionNames() {
- return FUNCTION_MAP.map().keySet();
+ public static NameMultimap<PinotFunction> getFunctionMap() {
+ return FUNCTION_MAP;
}
- /**
- * Returns {@code true} if the given function name is registered, {@code
false} otherwise.
- */
- public static boolean containsFunction(String functionName) {
- return FUNCTION_INFO_MAP.containsKey(canonicalize(functionName));
+ public static NameMultimap<SqlOperator> getOperatorMap() {
+ return OPERATOR_MAP;
}
- /**
- * Returns the {@link FunctionInfo} associated with the given function name
and number of parameters, 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.
- */
- @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;
+ private static void registerFunction(Method method, Set<String> alias,
boolean nullableParameters, boolean varArg) {
+ if (method.getAnnotation(Deprecated.class) == null) {
+ for (String name : alias) {
+ registerCalciteNamedFunctionMap(name, method, nullableParameters,
varArg);
}
- return functionInfoMap.get(VAR_ARG_KEY);
}
- return null;
}
- private static String canonicalize(String functionName) {
- return StringUtils.remove(functionName, '_').toLowerCase();
+ private static void registerFunction(Class<?> clazz, Set<String> alias,
boolean nullableParameters, boolean varArg) {
+ if (clazz.getAnnotation(Deprecated.class) == null) {
+ for (String name : alias) {
+ registerCalciteNamedFunctionMap(name, clazz, nullableParameters,
varArg);
+ }
+ }
+ }
+
+ private static void registerCalciteNamedFunctionMap(String name, Method
method, boolean nullableParameters,
+ boolean varArg) {
+ FUNCTION_MAP.put(name, new PinotScalarFunction(name, method,
nullableParameters, varArg));
+ }
+
+ private static void registerCalciteNamedFunctionMap(String name, Class<?>
clazz, boolean nullableParameters,
+ boolean varArg) {
+ try {
+ SqlReturnTypeInference returnTypeInference =
+ (SqlReturnTypeInference)
clazz.getField("RETURN_TYPE_INFERENCE").get(null);
+ SqlOperandTypeChecker operandTypeChecker =
+ (SqlOperandTypeChecker)
clazz.getField("OPERAND_TYPE_CHECKER").get(null);
+ for (Method method : clazz.getMethods()) {
+ if (method.getName().equals("eval")) {
Review Comment:
both are great suggestions/improvements. there's no reason, i simply feel:
1. there's no need to annotate additionally to all callable methods if we
already have the class annotated (might be verbose with lots of polymorphism)
2. for the special fields yeah we can use interface method stub. it is just
didn't occur to me that we need different ones within the same annotated
function class
--
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
