This is an automated email from the ASF dual-hosted git repository.
rongr pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new c124c6c2b7 [bugfix] fix placeholder resolution for scalar function
(#10050)
c124c6c2b7 is described below
commit c124c6c2b748d33c88549a5a5815534f6f3e28f4
Author: Rong Rong <[email protected]>
AuthorDate: Tue Jan 3 13:09:01 2023 -0800
[bugfix] fix placeholder resolution for scalar function (#10050)
* make a placeholder annotation for calcite function catalog
* adding dateTimeConvert placeholder
Co-authored-by: Rong Rong <[email protected]>
---
.../pinot/common/function/FunctionRegistry.java | 38 +++++++++++++++-------
.../function/InbuiltFunctionEvaluatorTest.java | 18 +++++++++-
.../pinot/query/runtime/QueryRunnerTest.java | 7 ++--
.../pinot/spi/annotations/ScalarFunction.java | 2 ++
4 files changed, 50 insertions(+), 15 deletions(-)
diff --git
a/pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java
b/pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java
index 86f7434204..4a5f2bb60d 100644
---
a/pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java
+++
b/pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java
@@ -72,10 +72,10 @@ public class FunctionRegistry {
boolean nullableParameters = scalarFunction.nullableParameters();
if (scalarFunctionNames.length > 0) {
for (String name : scalarFunctionNames) {
- FunctionRegistry.registerFunction(name, method,
nullableParameters);
+ FunctionRegistry.registerFunction(name, method,
nullableParameters, scalarFunction.isPlaceholder());
}
} else {
- FunctionRegistry.registerFunction(method, nullableParameters);
+ FunctionRegistry.registerFunction(method, nullableParameters,
scalarFunction.isPlaceholder());
}
}
}
@@ -94,15 +94,18 @@ public class FunctionRegistry {
/**
* Registers a method with the name of the method.
*/
- public static void registerFunction(Method method, boolean
nullableParameters) {
- registerFunction(method.getName(), method, nullableParameters);
+ public static void registerFunction(Method method, boolean
nullableParameters, boolean isPlaceholder) {
+ registerFunction(method.getName(), method, nullableParameters,
isPlaceholder);
}
/**
* Registers a method with the given function name.
*/
- public static void registerFunction(String functionName, Method method,
boolean nullableParameters) {
- registerFunctionInfoMap(functionName, method, nullableParameters);
+ public static void registerFunction(String functionName, Method method,
boolean nullableParameters,
+ boolean isPlaceholder) {
+ if (!isPlaceholder) {
+ registerFunctionInfoMap(functionName, method, nullableParameters);
+ }
registerCalciteNamedFunctionMap(functionName, method, nullableParameters);
}
@@ -161,33 +164,44 @@ public class FunctionRegistry {
*/
private static class PlaceholderScalarFunctions {
- @ScalarFunction(names = {"jsonExtractScalar", "json_extract_scalar"})
+ /**
+ * Noted that {@code dateTimeConvert} with String as first input is
actually supported.
+ *
+ * @see
org.apache.pinot.common.function.scalar.DateTimeConvert#dateTimeConvert(String,
String, String, String)
+ */
+ @ScalarFunction(names = {"dateTimeConvert", "date_time_convert"},
isPlaceholder = true)
+ public static String dateTimeConvert(long timeValueNumeric, String
inputFormatStr, String outputFormatStr,
+ String outputGranularityStr) {
+ throw new UnsupportedOperationException("Placeholder scalar function,
should not reach here");
+ }
+
+ @ScalarFunction(names = {"jsonExtractScalar", "json_extract_scalar"},
isPlaceholder = true)
public static Object jsonExtractScalar(String jsonFieldName, String
jsonPath, String resultsType) {
throw new UnsupportedOperationException("Placeholder scalar function,
should not reach here");
}
- @ScalarFunction(names = {"jsonExtractScalar", "json_extract_scalar"})
+ @ScalarFunction(names = {"jsonExtractScalar", "json_extract_scalar"},
isPlaceholder = true)
public static Object jsonExtractScalar(String jsonFieldName, String
jsonPath, String resultsType,
Object defaultValue) {
throw new UnsupportedOperationException("Placeholder scalar function,
should not reach here");
}
- @ScalarFunction(names = {"jsonExtractKey", "json_extract_key"})
+ @ScalarFunction(names = {"jsonExtractKey", "json_extract_key"},
isPlaceholder = true)
public static String jsonExtractKey(String jsonFieldName, String jsonPath)
{
throw new UnsupportedOperationException("Placeholder scalar function,
should not reach here");
}
- @ScalarFunction(names = {"textContains", "text_contains"})
+ @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");
}
- @ScalarFunction(names = {"textMatch", "text_match"})
+ @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");
}
- @ScalarFunction(names = {"jsonMatch", "json_match"})
+ @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");
}
diff --git
a/pinot-core/src/test/java/org/apache/pinot/core/data/function/InbuiltFunctionEvaluatorTest.java
b/pinot-core/src/test/java/org/apache/pinot/core/data/function/InbuiltFunctionEvaluatorTest.java
index 5c6835e293..82be9bcf52 100644
---
a/pinot-core/src/test/java/org/apache/pinot/core/data/function/InbuiltFunctionEvaluatorTest.java
+++
b/pinot-core/src/test/java/org/apache/pinot/core/data/function/InbuiltFunctionEvaluatorTest.java
@@ -30,6 +30,7 @@ import org.testng.annotations.Test;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertNull;
import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.fail;
public class InbuiltFunctionEvaluatorTest {
@@ -130,7 +131,7 @@ public class InbuiltFunctionEvaluatorTest {
throws Exception {
MyFunc myFunc = new MyFunc();
Method method =
myFunc.getClass().getDeclaredMethod("appendToStringAndReturn", String.class);
- FunctionRegistry.registerFunction(method, false);
+ FunctionRegistry.registerFunction(method, false, false);
String expression = "appendToStringAndReturn('test ')";
InbuiltFunctionEvaluator evaluator = new
InbuiltFunctionEvaluator(expression);
assertTrue(evaluator.getArguments().isEmpty());
@@ -157,6 +158,21 @@ public class InbuiltFunctionEvaluatorTest {
}
}
+ @Test
+ public void testPlaceholderFunctionShouldNotBeRegistered()
+ throws Exception {
+ GenericRow row = new GenericRow();
+ row.putValue("testColumn", "testValue");
+ String expression = "text_match(testColumn, 'pattern')";
+ try {
+ InbuiltFunctionEvaluator evaluator = new
InbuiltFunctionEvaluator(expression);
+ evaluator.evaluate(row);
+ fail();
+ } catch (Throwable t) {
+ assertTrue(t.getMessage().contains("text_match"));
+ }
+ }
+
public static class MyFunc {
String _baseString = "";
diff --git
a/pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/QueryRunnerTest.java
b/pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/QueryRunnerTest.java
index 37c0ed05bb..abb7d65e4d 100644
---
a/pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/QueryRunnerTest.java
+++
b/pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/QueryRunnerTest.java
@@ -266,9 +266,12 @@ public class QueryRunnerTest extends QueryRunnerTestBase {
new Object[]{"SELECT json_extract_key(col1, 'path') FROM a", "was
expecting (JSON String"},
// - PlaceholderScalarFunction registered will throw on intermediate
stage, but works on leaf stage.
+ // - checked "Illegal Json Path" as col1 is not actually a json
string, but the call is correctly triggered.
new Object[]{"SELECT CAST(jsonExtractScalar(col1, 'path', 'INT') AS
INT) FROM a", "Illegal Json Path"},
- new Object[]{"SELECT CAST(json_extract_scalar(a.col1, b.col2, 'INT')
AS INT) FROM a JOIN b ON a.col1 = b.col1",
- "PlaceholderScalarFunctions.jsonExtractScalar"},
+ // - checked function cannot be found b/c there's no intermediate
stage impl for json_extract_scalar
+ // TODO: re-enable this test once we have implemented constructor time
error pipe back.
+ // new Object[]{"SELECT CAST(json_extract_scalar(a.col1, b.col2,
'INT') AS INT)"
+ // + "FROM a JOIN b ON a.col1 = b.col1", "Cannot find function
with Name: json_extract_scalar"},
};
}
}
diff --git
a/pinot-spi/src/main/java/org/apache/pinot/spi/annotations/ScalarFunction.java
b/pinot-spi/src/main/java/org/apache/pinot/spi/annotations/ScalarFunction.java
index 6963c21b40..f0d14f7dd9 100644
---
a/pinot-spi/src/main/java/org/apache/pinot/spi/annotations/ScalarFunction.java
+++
b/pinot-spi/src/main/java/org/apache/pinot/spi/annotations/ScalarFunction.java
@@ -52,4 +52,6 @@ public @interface ScalarFunction {
// Whether the scalar function expects and can handle null arguments.
boolean nullableParameters() default false;
+
+ boolean isPlaceholder() default false;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
