[jira] [Commented] (PHOENIX-1889) Support alter/replace and drop functions

["James Taylor (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/PHOENIX-1889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14612261#comment-14612261 ] 

James Taylor commented on PHOENIX-1889:
---------------------------------------

Thanks for the patch, [~rajeshbabu]. It looks very good, but one question how are you handling the timestamp of the function metadata row? This code, in particular:
{code}
@@ -827,7 +827,7 @@ public class MetaDataEndpointImpl extends MetaDataProtocol implements Coprocesso
             disableWAL, multiTenant, storeNulls, viewType, viewIndexId, indexType, stats, baseColumnCount);
     }
 
-    private PFunction getFunction(RegionScanner scanner)
+    private PFunction getFunction(RegionScanner scanner, final boolean isReplace, List<Mutation> deleteMutationsForReplace)
             throws IOException, SQLException {
         List<Cell> results = Lists.newArrayList();
         scanner.next(results);
@@ -836,12 +836,15 @@ public class MetaDataEndpointImpl extends MetaDataProtocol implements Coprocesso
         }
         Cell[] functionKeyValues = new Cell[FUNCTION_KV_COLUMNS.size()];
         Cell[] functionArgKeyValues = new Cell[FUNCTION_ARG_KV_COLUMNS.size()];
-
         // Create PFunction based on KeyValues from scan
         Cell keyValue = results.get(0);
         byte[] keyBuffer = keyValue.getRowArray();
         int keyLength = keyValue.getRowLength();
         int keyOffset = keyValue.getRowOffset();
+        if(isReplace) {
+            deleteMutationsForReplace.add(new Delete(keyBuffer, keyOffset, keyLength, keyValue
+                    .getTimestamp()));
+        }
         PName tenantId = newPName(keyBuffer, keyOffset, keyLength);
         int tenantIdLength = (tenantId == null) ? 0 : tenantId.getBytes().length;
         if (tenantIdLength == 0) {
{code}
It should match the way we deal with other metadata in that we should issue the delete based on two factors:
- delete as of the SCN on the connection - 1 and create as of the SCN.
- if there's no SCN, then delete as of the current server timestamp - 1 (hopefully the current server timestamp is being used as the create timestamp too).
It might be working like that now, but I just wasn't sure in looking at the code

Also, how are we handling the case where the same jar name is used? This will be pretty typical, I suspect, especially using this new CREATE OR REPLACE syntax. Since the old jar cannot be unloaded, can we detect this and give an error?

One other small issue I just hit recently (not really related to this change but it's fine by me if you want to include it). If a function is used in a SQL statement that doesn't exist (my particular case was that the function exists, but I was passing too many arguments), then I got UNALLOWED_USER_DEFINED_FUNCTIONS exception: "User defined functions are configured to not be allowed. To allow configure...". We really should throw a FUNCTION_UNDEFINED exception instead. The UNALLOWED_USER_DEFINED_FUNCTIONS should be thrown if you try to create a UDF and the property that enables them is off.



> Support alter/replace and drop functions
> ----------------------------------------
>
>                 Key: PHOENIX-1889
>                 URL: https://issues.apache.org/jira/browse/PHOENIX-1889
>             Project: Phoenix
>          Issue Type: Sub-task
>            Reporter: Rajeshbabu Chintaguntla
>            Assignee: Rajeshbabu Chintaguntla
>             Fix For: 5.0.0, 4.5.0, 4.4.1
>
>         Attachments: PHOENIX-1889.patch
>
>




--
This message was sent by Atlassian JIRA
(v6.3.4#6332)