phoenix-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "James Taylor (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (PHOENIX-1889) Support alter/replace and drop functions
Date Thu, 02 Jul 2015 17:31:05 GMT

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

James Taylor edited comment on PHOENIX-1889 at 7/2/15 5:30 PM:
---------------------------------------------------------------

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.




was (Author: jamestaylor):
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)

Mime
View raw message