cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From tylerho...@apache.org
Subject git commit: Require arg types to disambiguate UDF drops
Date Wed, 03 Sep 2014 15:27:11 GMT
Repository: cassandra
Updated Branches:
  refs/heads/trunk ed0532c64 -> a88a78380


Require arg types to disambiguate UDF drops

Patch by Robert Stupp; reviewed by Tyler Hobbs for CASSANDRA-7812


Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/a88a7838
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/a88a7838
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/a88a7838

Branch: refs/heads/trunk
Commit: a88a78380ea26a25457a58a115195eb944bf0420
Parents: ed0532c
Author: Robert Stupp <snazy@snazy.de>
Authored: Wed Sep 3 10:26:30 2014 -0500
Committer: Tyler Hobbs <tyler@datastax.com>
Committed: Wed Sep 3 10:26:30 2014 -0500

----------------------------------------------------------------------
 CHANGES.txt                                     |  1 +
 src/java/org/apache/cassandra/cql3/Cql.g        | 13 +++-
 .../cassandra/cql3/functions/UDFunction.java    | 12 ++--
 .../cql3/statements/DropFunctionStatement.java  | 68 +++++++++++++++++---
 .../cassandra/service/MigrationManager.java     | 10 ++-
 test/unit/org/apache/cassandra/cql3/UFTest.java | 23 ++++++-
 6 files changed, 105 insertions(+), 22 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/a88a7838/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index babfa0e..281b63b 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 3.0
+ * Require arg types to disambiguate UDF drops (CASSANDRA-7812)
  * Do anticompaction in groups (CASSANDRA-6851)
  * Verify that UDF class methods are static (CASSANDRA-7781)
  * Support pure user-defined functions (CASSANDRA-7395, 7740)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/a88a7838/src/java/org/apache/cassandra/cql3/Cql.g
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/Cql.g b/src/java/org/apache/cassandra/cql3/Cql.g
index af2c239..8c40885 100644
--- a/src/java/org/apache/cassandra/cql3/Cql.g
+++ b/src/java/org/apache/cassandra/cql3/Cql.g
@@ -527,11 +527,22 @@ createFunctionStatement returns [CreateFunctionStatement expr]
 dropFunctionStatement returns [DropFunctionStatement expr]
     @init {
         boolean ifExists = false;
+        List<CQL3Type.Raw> argsTypes = new ArrayList<>();
+        boolean argsPresent = false;
     }
     : K_DROP K_FUNCTION
       (K_IF K_EXISTS { ifExists = true; } )?
       fn=functionName
-      { $expr = new DropFunctionStatement(fn, ifExists); }
+      (
+        '('
+          (
+            v=comparatorType { argsTypes.add(v); }
+            ( ',' v=comparatorType { argsTypes.add(v); } )*
+          )?
+        ')'
+        { argsPresent = true; }
+      )?
+      { $expr = new DropFunctionStatement(fn, argsTypes, argsPresent, ifExists); }
     ;
 
 /**

http://git-wip-us.apache.org/repos/asf/cassandra/blob/a88a7838/src/java/org/apache/cassandra/cql3/functions/UDFunction.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/functions/UDFunction.java b/src/java/org/apache/cassandra/cql3/functions/UDFunction.java
index 558fb72..aeefe41 100644
--- a/src/java/org/apache/cassandra/cql3/functions/UDFunction.java
+++ b/src/java/org/apache/cassandra/cql3/functions/UDFunction.java
@@ -138,11 +138,15 @@ public abstract class UDFunction extends AbstractFunction
         return new Mutation(Keyspace.SYSTEM_KS, kv.decompose(name.namespace, name.name));
     }
 
-    // TODO: we should allow removing just one function, not all functions having a given
name
-    public static Mutation dropFromSchema(long timestamp, FunctionName fun)
+    public Mutation toSchemaDrop(long timestamp)
     {
-        Mutation mutation = makeSchemaMutation(fun);
-        mutation.delete(SystemKeyspace.SCHEMA_FUNCTIONS_CF, timestamp);
+        Mutation mutation = makeSchemaMutation(name);
+        ColumnFamily cf = mutation.addOrGet(SystemKeyspace.SCHEMA_FUNCTIONS_CF);
+
+        Composite prefix = CFMetaData.SchemaFunctionsCf.comparator.make(computeSignature(argTypes));
+        int ldt = (int) (System.currentTimeMillis() / 1000);
+        cf.addAtom(new RangeTombstone(prefix, prefix.end(), timestamp, ldt));
+
         return mutation;
     }
 

http://git-wip-us.apache.org/repos/asf/cassandra/blob/a88a7838/src/java/org/apache/cassandra/cql3/statements/DropFunctionStatement.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/statements/DropFunctionStatement.java b/src/java/org/apache/cassandra/cql3/statements/DropFunctionStatement.java
index 4c963a8..78c8607 100644
--- a/src/java/org/apache/cassandra/cql3/statements/DropFunctionStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/DropFunctionStatement.java
@@ -17,10 +17,13 @@
  */
 package org.apache.cassandra.cql3.statements;
 
+import java.util.ArrayList;
 import java.util.List;
 
 import org.apache.cassandra.auth.Permission;
+import org.apache.cassandra.cql3.CQL3Type;
 import org.apache.cassandra.cql3.functions.*;
+import org.apache.cassandra.db.marshal.AbstractType;
 import org.apache.cassandra.exceptions.InvalidRequestException;
 import org.apache.cassandra.exceptions.RequestValidationException;
 import org.apache.cassandra.exceptions.UnauthorizedException;
@@ -35,10 +38,17 @@ public final class DropFunctionStatement extends SchemaAlteringStatement
 {
     private final FunctionName functionName;
     private final boolean ifExists;
+    private final List<CQL3Type.Raw> argRawTypes;
+    private final boolean argsPresent;
 
-    public DropFunctionStatement(FunctionName functionName, boolean ifExists)
+    public DropFunctionStatement(FunctionName functionName,
+                                 List<CQL3Type.Raw> argRawTypes,
+                                 boolean argsPresent,
+                                 boolean ifExists)
     {
         this.functionName = functionName;
+        this.argRawTypes = argRawTypes;
+        this.argsPresent = argsPresent;
         this.ifExists = ifExists;
     }
 
@@ -68,18 +78,58 @@ public final class DropFunctionStatement extends SchemaAlteringStatement
     public boolean announceMigration(boolean isLocalOnly) throws RequestValidationException
     {
         List<Function> olds = Functions.find(functionName);
-        if (olds == null || olds.isEmpty())
+
+        if (!argsPresent && olds != null && olds.size() > 1)
+            throw new InvalidRequestException(String.format("'DROP FUNCTION %s' matches multiple
function definitions; " +
+                                                            "specify the argument types by
issuing a statement like " +
+                                                            "'DROP FUNCTION %s (type, type,
...)'. Hint: use cqlsh " +
+                                                            "'DESCRIBE FUNCTION %s' command
to find all overloads",
+                                                            functionName, functionName, functionName));
+
+        List<AbstractType<?>> argTypes = new ArrayList<>(argRawTypes.size());
+        for (CQL3Type.Raw rawType : argRawTypes)
+        {
+            // We have no proper keyspace to give, which means that this will break (NPE
currently)
+            // for UDT: #7791 is open to fix this
+            argTypes.add(rawType.prepare(null).getType());
+        }
+
+        Function old;
+        if (argsPresent)
+        {
+            old = Functions.find(functionName, argTypes);
+            if (old == null)
+            {
+                if (ifExists)
+                    return false;
+                // just build a nicer error message
+                StringBuilder sb = new StringBuilder();
+                for (CQL3Type.Raw rawType : argRawTypes)
+                {
+                    if (sb.length() > 0)
+                        sb.append(", ");
+                    sb.append(rawType);
+                }
+                throw new InvalidRequestException(String.format("Cannot drop non existing
function '%s(%s)'",
+                                                                functionName, sb));
+            }
+        }
+        else
         {
-            if (ifExists)
-                return false;
-            throw new InvalidRequestException(String.format("Cannot drop non existing function
'%s'", functionName));
+            if (olds == null || olds.isEmpty())
+            {
+                if (ifExists)
+                    return false;
+                throw new InvalidRequestException(String.format("Cannot drop non existing
function '%s'", functionName));
+            }
+            old = olds.get(0);
         }
 
-        for (Function f : olds)
-            if (f.isNative())
-                throw new InvalidRequestException(String.format("Cannot drop function '%s'
because it has native overloads", functionName));
+        if (old.isNative())
+            throw new InvalidRequestException(String.format("Cannot drop function '%s' because
it is a " +
+                                                            "native (built-in) function",
functionName));
 
-        MigrationManager.announceFunctionDrop(functionName, isLocalOnly);
+        MigrationManager.announceFunctionDrop((UDFunction)old, isLocalOnly);
         return true;
     }
 }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/a88a7838/src/java/org/apache/cassandra/service/MigrationManager.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/service/MigrationManager.java b/src/java/org/apache/cassandra/service/MigrationManager.java
index b0cf79d..4f03483 100644
--- a/src/java/org/apache/cassandra/service/MigrationManager.java
+++ b/src/java/org/apache/cassandra/service/MigrationManager.java
@@ -38,14 +38,11 @@ import org.apache.cassandra.config.CFMetaData;
 import org.apache.cassandra.config.KSMetaData;
 import org.apache.cassandra.config.UTMetaData;
 import org.apache.cassandra.config.Schema;
-import org.apache.cassandra.cql3.functions.FunctionName;
 import org.apache.cassandra.cql3.functions.UDFunction;
 import org.apache.cassandra.db.*;
 import org.apache.cassandra.db.marshal.UserType;
 import org.apache.cassandra.exceptions.AlreadyExistsException;
 import org.apache.cassandra.exceptions.ConfigurationException;
-import org.apache.cassandra.exceptions.InvalidRequestException;
-import org.apache.cassandra.exceptions.SyntaxException;
 import org.apache.cassandra.gms.*;
 import org.apache.cassandra.io.IVersionedSerializer;
 import org.apache.cassandra.io.util.DataOutputPlus;
@@ -374,10 +371,11 @@ public class MigrationManager
         announce(addSerializedKeyspace(UTMetaData.dropFromSchema(droppedType, FBUtilities.timestampMicros()),
droppedType.keyspace), announceLocally);
     }
 
-    public static void announceFunctionDrop(FunctionName fun, boolean announceLocally)
+    public static void announceFunctionDrop(UDFunction udf, boolean announceLocally)
     {
-        logger.info(String.format("Drop Function '%s'", fun));
-        announce(UDFunction.dropFromSchema(FBUtilities.timestampMicros(), fun), announceLocally);
+        Mutation mutation = udf.toSchemaDrop(FBUtilities.timestampMicros());
+        logger.info(String.format("Drop Function overload '%s' args '%s'", udf.name(), udf.argTypes()));
+        announce(mutation, announceLocally);
     }
 
     public static void announceNewFunction(UDFunction udf, boolean announceLocally)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/a88a7838/test/unit/org/apache/cassandra/cql3/UFTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/cql3/UFTest.java b/test/unit/org/apache/cassandra/cql3/UFTest.java
index 84161b2..60a7a68 100644
--- a/test/unit/org/apache/cassandra/cql3/UFTest.java
+++ b/test/unit/org/apache/cassandra/cql3/UFTest.java
@@ -19,8 +19,6 @@ package org.apache.cassandra.cql3;
 
 import org.junit.Test;
 
-import org.apache.cassandra.exceptions.InvalidRequestException;
-
 public class UFTest extends CQLTester
 {
     public static Double sin(Double val)
@@ -141,6 +139,10 @@ public class UFTest extends CQLTester
         assertInvalid("DROP FUNCTION foo::sin");
         // but don't complain with "IF EXISTS"
         execute("DROP FUNCTION IF EXISTS foo::sin");
+
+        // can't drop native functions
+        assertInvalid("DROP FUNCTION dateof");
+        assertInvalid("DROP FUNCTION uuid");
     }
 
     @Test
@@ -186,5 +188,22 @@ public class UFTest extends CQLTester
         assertEmpty(execute("SELECT v FROM %s WHERE k = overloaded((ascii)?)", "foo"));
         // And since varchar == text, this should work too
         assertEmpty(execute("SELECT v FROM %s WHERE k = overloaded((varchar)?)", "foo"));
+
+        // no such functions exist...
+        assertInvalid("DROP FUNCTION overloaded(boolean)");
+        assertInvalid("DROP FUNCTION overloaded(bigint)");
+
+        // 'overloaded' has multiple overloads - so it has to fail (CASSANDRA-7812)
+        assertInvalid("DROP FUNCTION overloaded");
+        execute("DROP FUNCTION overloaded(varchar)");
+        assertInvalid("SELECT v FROM %s WHERE k = overloaded((text)?)", "foo");
+        execute("DROP FUNCTION overloaded(text, text)");
+        assertInvalid("SELECT v FROM %s WHERE k = overloaded((text)?,(text)?)", "foo", "bar");
+        execute("DROP FUNCTION overloaded(ascii)");
+        assertInvalid("SELECT v FROM %s WHERE k = overloaded((ascii)?)", "foo");
+        // single-int-overload must still work
+        assertRows(execute("SELECT v FROM %s WHERE k = overloaded((int)?)", 3), row(1));
+        // overloaded has just one overload now - so the following DROP FUNCTION is not ambigious
(CASSANDRA-7812)
+        execute("DROP FUNCTION overloaded");
     }
 }


Mime
View raw message