[
https://issues.apache.org/jira/browse/PHOENIX-538?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14508532#comment-14508532
]
James Taylor commented on PHOENIX-538:
--------------------------------------
Overall, looks good, [~rajeshbabu]. Very nice work. A few minor items and a
couple of questions:
- How about renaming PCommon to PMetaData or PMetaDataEntity?
- Are List<byte[]> keys guaranteed to be ordered? If not, best to sort them
before getting the lock to prevent potential for a deadlock.
{code}
+ private List<PFunction> doGetFunctions(List<byte[]> keys, long
clientTimeStamp, List<RowLock> rowLocks) throws IOException, SQLException {
+ Cache<ImmutableBytesPtr, PCommon> metaDataCache =
+ GlobalCache.getInstance(this.env).getMetaDataCache();
+ HRegion region = env.getRegion();
+ /*
+ * Lock directly on key, though it may be an index table. This will
just prevent a table
+ * from getting rebuilt too often.
+ */
+ final boolean wasLocked = (rowLocks != null);
+ try {
+ if (!wasLocked) {
+ rowLocks = new ArrayList<HRegion.RowLock>(keys.size());
+ for(int i = 0; i< keys.size(); i++) {
{code}
- File another followup JIRA for supporting the
DatabaseMetaData.getFunctionColumns() and getFunctions() calls.
- One potential issue with UDFs is that you don't know if users will hold state
in their UDF implementation in member variables. Because of that, it's probably
safest to override the UDFExpression.getDeterminism() method to return
PER_INVOCATION which will force the client to create a new Expression tree
anytime a UDF is used (basically, each thread will have its own copy when an
UPSERT SELECT is running, so member variables wouldn't cause a problem).
- Any thoughts about how to handle PHOENIX-1907 for now? Just doc it? Or should
we disallow UDFs from being used in a functional index? [~tdsilva] - any ideas
on this? I think ideally, we'd want to disable the functional index if a new
implementation of a UDF is loaded. We'd likely need a new column on
system.catalog to track this.
- I'm not seeing any unit tests around CREATE OR REPLACE FUNCTION. Is that
implemented yet? How would that deal with unloading an existing jar? Or would
it be required to place it in a new jar. Is that enforced?
Otherwise, it looks good to me. Anyone else want to review?
> 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)
