New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add SCALAR type into TransformFunctionType #5544
Add SCALAR type into TransformFunctionType #5544
Conversation
Inside query engine, scalar function is modeled as TransformFunction (ScalarTransformFunctionWrapper) Add SCALAR type into TransformFunctionType so that FunctionDefinitionRegistry can identify scalar function as transform function
7942d78
to
cc80829
Compare
Codecov Report
@@ Coverage Diff @@
## master #5544 +/- ##
=========================================
Coverage ? 66.29%
=========================================
Files ? 1105
Lines ? 56877
Branches ? 8506
=========================================
Hits ? 37709
Misses ? 16378
Partials ? 2790
Continue to review full report at Codecov.
|
@@ -70,6 +73,9 @@ public static TransformFunctionType getTransformFunctionType(String functionName | |||
try { | |||
return TransformFunctionType.valueOf(upperCaseFunctionName); | |||
} catch (Exception e) { | |||
if (FunctionRegistry.getFunctionByName(functionName) != null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks a bit hacky especially if FunctionRegistry starts getting used for something else as well (in addition to scalar functions). May be update the Javadoc of FunctionRegistry to state it is for scalar functions?
Currently it says in-built Pinot functions which is misleading since today all functions are in-built functions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@siddharthteotia Updated the doc, and also added a TODO to merge FunctionRegistry and FunctionDefinitionRegistry to provide one single registry for all functions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome. Thanks!
Description
Inside query engine, scalar function is modeled as TransformFunction (ScalarTransformFunctionWrapper)
Add SCALAR type into TransformFunctionType so that FunctionDefinitionRegistry can identify scalar function as transform function