[
https://issues.apache.org/jira/browse/PHOENIX-538?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14502504#comment-14502504
]
James Taylor commented on PHOENIX-538:
--------------------------------------
[~rajeshbabu] - this is looking very good. Nice work. Main thing it needs is
lots more tests. Is there just the UserDefinedFunctionsIT for testing current?
Here would be some useful tests to add:
- Create a functional index using a UDF and make sure it's used when it should
be (check query plan) and that the correct values are used in a query.
- Is it allowed for a UDF to be replaced with a new/different implementation?
It should be allowed (at least eventually), but then you'll need to know which
functional indexes are using this function and rebuild them (as the returned
value may be different now).
- Add class loader related tests such as different tenant IDs using the same
function with different implementations. Also, a tenant overriding a globally
defined UDF (is that allowed?).
- Add negative tests around defining an already defined UDF, defining the same
UDF for the same tenant ID (and/or globally), but with a different class name.
What should happen?
- Add tests that use a UDF in a where clause (if you have that already, sorry I
missed it).
Also, some minor stuff I noticed:
Instead of doing a containsKey() call and a get() call, just do a get() call to
prevent multiple map look ups and throw if the get() returns null:
{code}
+
+ @Override
+ public PFunction resolveFunction(String functionName) throws
SQLException {
+ if(functionMap.containsKey(functionName)) return
functionMap.get(functionName);
+ throw new FunctionNotFoundException(functionName);
+ }
+
{code}
Don't check for a null PFunction, as your resolveFunction guarantees a non null
value:
{code}
+ PFunction function = null;
+ if(node instanceof UDFParseNode) {
+ function = context.getResolver().resolveFunction(node.getName());
+ if(function == null) {
+ throw new FunctionNotFoundException(node.getName());
+ }
+ BuiltInFunctionInfo info = new BuiltInFunctionInfo(function);
+ node = new UDFParseNode(node.getName(), node.getChildren(), info);
+ }
{code}
> Support UDFs
> ------------
>
> Key: PHOENIX-538
> URL: https://issues.apache.org/jira/browse/PHOENIX-538
> Project: Phoenix
> Issue Type: Task
> Reporter: James Taylor
> Assignee: Rajeshbabu Chintaguntla
> Fix For: 5.0.0, 4.4.0
>
> Attachments: PHOENIX-538-wip.patch, PHOENIX-538_v1.patch,
> PHOENIX-538_v2.patch, PHOENIX-538_v3.patch
>
>
> Phoenix allows built-in functions to be added (as described
> [here](http://phoenix-hbase.blogspot.com/2013/04/how-to-add-your-own-built-in-function.html))
> with the restriction that they must be in the phoenix jar. We should improve
> on this and allow folks to declare new functions through a CREATE FUNCTION
> command like this:
> CREATE FUNCTION mdHash(anytype)
> RETURNS binary(16)
> LOCATION 'hdfs://path-to-my-jar' 'com.me.MDHashFunction'
> Since HBase supports loading jars dynamically, this would not be too
> difficult. The function implementation class would be required to extend our
> ScalarFunction base class. Here's how I could see it being implemented:
> * modify the phoenix grammar to support the new CREATE FUNCTION syntax
> * create a new UTFParseNode class to capture the parse state
> * add a new method to the MetaDataProtocol interface
> * add a new method in ConnectionQueryServices to invoke the MetaDataProtocol
> method
> * add a new method in MetaDataClient to invoke the ConnectionQueryServices
> method
> * persist functions in a new "SYSTEM.FUNCTION" table
> * add a new client-side representation to cache functions called PFunction
> * modify ColumnResolver to dynamically resolve a function in the same way we
> dynamically resolve and load a table
> * create and register a new ExpressionType called UDFExpression
> * at parse time, check for the function name in the built in list first (as
> is currently done), and if not found in the PFunction cache. If not found
> there, then use the new UDFExpression as a placeholder and have the
> ColumnResolver attempt to resolve it at compile time and throw an error if
> unsuccessful.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
