reallocf commented on a change in pull request #5326:
URL: https://github.com/apache/incubator-pinot/pull/5326#discussion_r422548708
##########
File path:
pinot-core/src/test/java/org/apache/pinot/core/data/function/DefaultFunctionEvaluatorTest.java
##########
@@ -54,49 +54,73 @@ public void testExpressionWithColumn()
@Test
public void testExpressionWithConstant()
throws Exception {
- FunctionRegistry
-
.registerStaticFunction(MyFunc.class.getDeclaredMethod("daysSinceEpoch",
String.class, String.class));
+ MyFunc myFunc = new MyFunc();
+ FunctionRegistry functionRegistry = new FunctionRegistry(
+
Lists.newArrayList(myFunc.getClass().getDeclaredMethod("daysSinceEpoch",
String.class, String.class)));
String input = "1980-01-01";
String format = "yyyy-MM-dd";
String expression = String.format("daysSinceEpoch('%s', '%s')", input,
format);
- DefaultFunctionEvaluator evaluator = new
DefaultFunctionEvaluator(expression);
+ DefaultFunctionEvaluator evaluator = new
DefaultFunctionEvaluator(expression, functionRegistry);
Assert.assertTrue(evaluator.getArguments().isEmpty());
GenericRow row = new GenericRow();
Object result = evaluator.evaluate(row);
- Assert.assertEquals(result, MyFunc.daysSinceEpoch(input, format));
+ Assert.assertEquals(result, myFunc.daysSinceEpoch(input, format));
}
@Test
public void testMultiFunctionExpression()
throws Exception {
-
FunctionRegistry.registerStaticFunction(MyFunc.class.getDeclaredMethod("reverseString",
String.class));
- FunctionRegistry
-
.registerStaticFunction(MyFunc.class.getDeclaredMethod("daysSinceEpoch",
String.class, String.class));
+ MyFunc myFunc = new MyFunc();
+ FunctionRegistry functionRegistry = new FunctionRegistry(Lists
+ .newArrayList(myFunc.getClass().getDeclaredMethod("reverseString",
String.class),
+ myFunc.getClass().getDeclaredMethod("daysSinceEpoch",
String.class, String.class)));
String input = "1980-01-01";
- String reversedInput = MyFunc.reverseString(input);
+ String reversedInput = myFunc.reverseString(input);
String format = "yyyy-MM-dd";
String expression = String.format("daysSinceEpoch(reverseString('%s'),
'%s')", reversedInput, format);
- DefaultFunctionEvaluator evaluator = new
DefaultFunctionEvaluator(expression);
+ DefaultFunctionEvaluator evaluator = new
DefaultFunctionEvaluator(expression, functionRegistry);
Assert.assertTrue(evaluator.getArguments().isEmpty());
GenericRow row = new GenericRow();
Object result = evaluator.evaluate(row);
- Assert.assertEquals(result, MyFunc.daysSinceEpoch(input, format));
+ Assert.assertEquals(result, myFunc.daysSinceEpoch(input, format));
}
- private static class MyFunc {
- static String reverseString(String input) {
- return new StringBuilder(input).reverse().toString();
- }
+ @Test
+ public void testStateSharedBetweenRowsForExecution()
+ throws Exception {
Review comment:
This test basically just confirms that the internal state of the
FunctionRegistry is shared between each row. I agree with the current
implementation it's fairly self-explanatory, but you can imagine an
implementation where the internals of the FunctionRegistry are different for
each row. This is to make sure we don't somehow regress to that, because then
we'd see a big performance hit for creating a SDF for each row.
But can definitely remove if it just feels like clutter to you ☺️
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
