[
https://issues.apache.org/jira/browse/PHOENIX-538?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14507816#comment-14507816
]
Rajeshbabu Chintaguntla commented on PHOENIX-538:
-------------------------------------------------
[~jamestaylor] Thanks for the inputs and review.
Attached the patch addressing them. It's rebased to latest code. In this patch
added drop function support as well.
All tests for UDF are in UserDefinedFunctionsIT only James.
bq. 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.
Added the test case with both global and local functional indexes.
bq. 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).
James this is not supported currently. I will raise separate subtask for this
and work on it.
bq. 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?).
Yes it's allowed and added tests for this.
bq. 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?
FunctionAlreadyExists should be thrown. Added the test.
bq. Add tests that use a UDF in a where clause (if you have that already, sorry
I missed it).
Yes I have added in earlier patch itself
bq. 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
done.
bq. Don't check for a null PFunction, as your resolveFunction guarantees a non
null value:
done.
bq. One more thing that'd be good to consider would be to use the metaDataCache
to store both functions and table definitions. Configuring one cache rather
than two is easier to manage and you'll be able to reuse the code (just create
a couple of key types). Same with the client-side - you could store them in the
same client-side cache (in addition to the way you're reusing the code there
which is good). It just makes things easier to manage: single server-side
metadata cache and single client side metadata cache.
Avoiding configuring two caches for tables and functions, unified them at
server side. Please check GlobalCache. At client side keeing functions,tables
together in metaDataCache.
bq. Other tests that are necessary are ones that test the caching layer on the
client and server by opening two separate connections in a testt
Added the tests for the list.
Please review.
Thank you very much.
> 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-538_v4.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)
