[
https://issues.apache.org/jira/browse/PHOENIX-538?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14509568#comment-14509568
]
Samarth Jain commented on PHOENIX-538:
--------------------------------------
In general, I think the synchronization model could be simplified. For example
- I don't think you need the KeyLocker because you are already using a
concurrent map. Also there is a bug in your code (added as comment). See the
code snippet below that simplifies the synchronization model:
{code}
+ if (jarPath == null || jarPath.isEmpty() ||
config.get(DYNAMIC_JARS_DIR_KEY) != null
+ && (parent != null &&
parent.equals(config.get(DYNAMIC_JARS_DIR_KEY)))) {
+ cl = tenantIdSpecificCls.get(tenantId);
+ if (cl == null) {
+ final DynamicClassLoader newCl = new
DynamicClassLoader(config, UDFExpression.class.getClassLoader());
+ // Cache class loader as a weak value, will be GC'ed when
no reference left
+ DynamicClassLoader prev =
tenantIdSpecificCls.putIfAbsent(tenantId, newCl);
+ if (prev != null) {
+ cl = prev;
+ }
+ }
+ } else {
+ cl = pathSpecificCls.get(jarPath); //----------> there was a
bug here. Instead of jarpath you had tenantId here.
+ if (cl == null) {
+ Configuration conf = HBaseConfiguration.create(config);
+ conf.set(DYNAMIC_JARS_DIR_KEY, parent);
+ DynamicClassLoader newCl = new DynamicClassLoader(conf,
UDFExpression.class.getClassLoader());
+ // Cache class loader as a weak value, will be GC'ed when
no reference left
+ DynamicClassLoader prev =
pathSpecificCls.putIfAbsent(jarPath, cl);
+ if (prev != null) {
+ cl = prev;
+ }
+ }
+ }
+ return cl;
{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-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)
