Return-Path: X-Original-To: apmail-cassandra-commits-archive@www.apache.org Delivered-To: apmail-cassandra-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id EDC1E173B7 for ; Mon, 13 Oct 2014 13:57:41 +0000 (UTC) Received: (qmail 14921 invoked by uid 500); 13 Oct 2014 13:57:41 -0000 Delivered-To: apmail-cassandra-commits-archive@cassandra.apache.org Received: (qmail 14887 invoked by uid 500); 13 Oct 2014 13:57:41 -0000 Mailing-List: contact commits-help@cassandra.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@cassandra.apache.org Delivered-To: mailing list commits@cassandra.apache.org Received: (qmail 14876 invoked by uid 99); 13 Oct 2014 13:57:41 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 13 Oct 2014 13:57:41 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id 80EAC90D56F; Mon, 13 Oct 2014 13:57:41 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: slebresne@apache.org To: commits@cassandra.apache.org Message-Id: <718d946a8f0647e887084ba9783a1ad5@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: git commit: Remove UDF-as-class functionality Date: Mon, 13 Oct 2014 13:57:41 +0000 (UTC) Repository: cassandra Updated Branches: refs/heads/trunk 0de0b8c03 -> 9333f86cf Remove UDF-as-class functionality patch by snazy; reviewed by slebresne for CASSANDRA-8063 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/9333f86c Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/9333f86c Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/9333f86c Branch: refs/heads/trunk Commit: 9333f86cf0acfdce75ebcd1ff38263e3d199efc7 Parents: 0de0b8c Author: Sylvain Lebresne Authored: Mon Oct 13 15:57:01 2014 +0200 Committer: Sylvain Lebresne Committed: Mon Oct 13 15:57:01 2014 +0200 ---------------------------------------------------------------------- CHANGES.txt | 9 +- pylib/cqlshlib/cql3handling.py | 9 +- src/java/org/apache/cassandra/cql3/Cql.g | 19 +-- .../cql3/functions/ReflectionBasedUDF.java | 127 ------------------- .../cassandra/cql3/functions/UDFunction.java | 1 - test/unit/org/apache/cassandra/cql3/UFTest.java | 98 +++----------- 6 files changed, 23 insertions(+), 240 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/9333f86c/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index b6a3766..cd3950f 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,21 +1,16 @@ 3.0 * Keep sstable levels when bootstrapping (CASSANDRA-7460) * Add Sigar library and perform basic OS settings check on startup (CASSANDRA-7838) - * Support for scripting languages in user-defined functions (CASSANDRA-7526) * Support for aggregation functions (CASSANDRA-4914) - * Improve query to read paxos table on propose (CASSANDRA-7929) * Remove cassandra-cli (CASSANDRA-7920) - * Optimize java source-based UDF invocation (CASSANDRA-7924) * Accept dollar quoted strings in CQL (CASSANDRA-7769) * Make assassinate a first class command (CASSANDRA-7935) * Support IN clause on any clustering column (CASSANDRA-4762) * Improve compaction logging (CASSANDRA-7818) * Remove YamlFileNetworkTopologySnitch (CASSANDRA-7917) - * Support Java source code for user-defined functions (CASSANDRA-7562) - * 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) + * Support pure user-defined functions (CASSANDRA-7395, 7526, 7562, 7740, 7781, 7929, + 7924, 7812, 8063) * Permit configurable timestamps with cassandra-stress (CASSANDRA-7416) * Move sstable RandomAccessReader to nio2, which allows using the FILE_SHARE_DELETE flag on Windows (CASSANDRA-4050) http://git-wip-us.apache.org/repos/asf/cassandra/blob/9333f86c/pylib/cqlshlib/cql3handling.py ---------------------------------------------------------------------- diff --git a/pylib/cqlshlib/cql3handling.py b/pylib/cqlshlib/cql3handling.py index 69fc277..6980216 100644 --- a/pylib/cqlshlib/cql3handling.py +++ b/pylib/cqlshlib/cql3handling.py @@ -1004,14 +1004,7 @@ syntax_rules += r''' ( "," [newcolname]= )* )? ")" )? "RETURNS" - ( - ("LANGUAGE" "AS" - ( - - ) - ) - | ("USING" ) - ) + "LANGUAGE" "AS" ; ''' http://git-wip-us.apache.org/repos/asf/cassandra/blob/9333f86c/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 2ec9746..81f7d25 100644 --- a/src/java/org/apache/cassandra/cql3/Cql.g +++ b/src/java/org/apache/cassandra/cql3/Cql.g @@ -493,8 +493,6 @@ createFunctionStatement returns [CreateFunctionStatement expr] boolean ifNotExists = false; boolean deterministic = true; - String language = "class"; - String bodyOrClassName = null; List argsNames = new ArrayList<>(); List argsTypes = new ArrayList<>(); } @@ -509,19 +507,10 @@ createFunctionStatement returns [CreateFunctionStatement expr] ( ',' k=cident v=comparatorType { argsNames.add(k); argsTypes.add(v); } )* )? ')' - K_RETURNS - rt=comparatorType - ( - ( K_USING cls = STRING_LITERAL { bodyOrClassName = $cls.text; } ) - | ( K_LANGUAGE l = IDENT { language=$l.text; } K_AS - ( - ( body = STRING_LITERAL - { bodyOrClassName = $body.text; } - ) - ) - ) - ) - { $expr = new CreateFunctionStatement(fn, language.toLowerCase(), bodyOrClassName, deterministic, argsNames, argsTypes, rt, orReplace, ifNotExists); } + K_RETURNS rt = comparatorType + K_LANGUAGE language = IDENT + K_AS body = STRING_LITERAL + { $expr = new CreateFunctionStatement(fn, $language.text.toLowerCase(), $body.text, deterministic, argsNames, argsTypes, rt, orReplace, ifNotExists); } ; dropFunctionStatement returns [DropFunctionStatement expr] http://git-wip-us.apache.org/repos/asf/cassandra/blob/9333f86c/src/java/org/apache/cassandra/cql3/functions/ReflectionBasedUDF.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/functions/ReflectionBasedUDF.java b/src/java/org/apache/cassandra/cql3/functions/ReflectionBasedUDF.java deleted file mode 100644 index 911537f..0000000 --- a/src/java/org/apache/cassandra/cql3/functions/ReflectionBasedUDF.java +++ /dev/null @@ -1,127 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.cassandra.cql3.functions; - -import java.lang.invoke.MethodHandle; -import java.lang.invoke.MethodHandles; -import java.lang.invoke.MethodType; -import java.nio.ByteBuffer; -import java.util.Arrays; -import java.util.List; - -import org.apache.cassandra.cql3.*; -import org.apache.cassandra.db.marshal.AbstractType; -import org.apache.cassandra.exceptions.InvalidRequestException; - -/** - * User-defined function using a method in a class loaded on the classpath by - * reflection. - * - * This is used when the LANGUAGE of the UDF definition is "class". - */ -final class ReflectionBasedUDF extends UDFunction -{ - private final MethodHandle method; - - ReflectionBasedUDF(FunctionName name, - List argNames, - List> argTypes, - AbstractType returnType, - String language, - String body, - boolean deterministic) - throws InvalidRequestException - { - super(name, argNames, argTypes, returnType, language, body, deterministic); - assert language.equals("class"); - this.method = resolveMethod(); - } - - private MethodHandle resolveMethod() throws InvalidRequestException - { - Class jReturnType = javaType(returnType); - Class[] paramTypes = javaParamTypes(argTypes); - - String className; - String methodName; - int i = body.indexOf('#'); - if (i != -1) - { - methodName = body.substring(i + 1); - className = body.substring(0, i); - } - else - { - methodName = name.name; - className = body; - } - try - { - Class cls = Class.forName(className, false, Thread.currentThread().getContextClassLoader()); - - MethodHandles.Lookup handles = MethodHandles.lookup(); - MethodType methodType = MethodType.methodType(jReturnType, paramTypes); - MethodHandle handle = handles.findStatic(cls, methodName, methodType); - - return handle; - } - catch (ClassNotFoundException e) - { - throw new InvalidRequestException("Class " + className + " does not exist"); - } - catch (NoSuchMethodException e) - { - throw new InvalidRequestException("Method 'public static " + jReturnType.getName() + ' ' + - className + '.' + methodName + '(' + Arrays.toString(paramTypes) + - ")' does not exist - check for static, argument types and return type"); - } - catch (IllegalAccessException e) - { - throw new InvalidRequestException("Method " + className + '.' + methodName + '(' + Arrays.toString(paramTypes) + ") is not accessible"); - } - } - - public ByteBuffer execute(List parameters) throws InvalidRequestException - { - Object[] parms = new Object[argTypes.size()]; - for (int i = 0; i < parms.length; i++) - { - ByteBuffer bb = parameters.get(i); - if (bb != null) - parms[i] = argTypes.get(i).compose(bb); - } - - Object result; - try - { - result = method.invokeWithArguments(parms); - @SuppressWarnings("unchecked") ByteBuffer r = result != null ? ((AbstractType) returnType).decompose(result) : null; - return r; - } - catch (VirtualMachineError e) - { - // handle OutOfMemoryError and other fatals not here! - throw e; - } - catch (Throwable e) - { - logger.error("Invocation of function '{}' failed", this, e); - throw new InvalidRequestException("Invocation of function '" + this + "' failed: " + e); - } - } -} http://git-wip-us.apache.org/repos/asf/cassandra/blob/9333f86c/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 8c51b9e..3741008 100644 --- a/src/java/org/apache/cassandra/cql3/functions/UDFunction.java +++ b/src/java/org/apache/cassandra/cql3/functions/UDFunction.java @@ -82,7 +82,6 @@ public abstract class UDFunction extends AbstractFunction implements ScalarFunct { switch (language) { - case "class": return new ReflectionBasedUDF(name, argNames, argTypes, returnType, language, body, deterministic); case "java": return JavaSourceUDFFactory.buildUDF(name, argNames, argTypes, returnType, body, deterministic); default: return new ScriptBasedUDF(name, argNames, argTypes, returnType, language, body, deterministic); } http://git-wip-us.apache.org/repos/asf/cassandra/blob/9333f86c/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 46db578..95bede4 100644 --- a/test/unit/org/apache/cassandra/cql3/UFTest.java +++ b/test/unit/org/apache/cassandra/cql3/UFTest.java @@ -27,59 +27,6 @@ import org.apache.cassandra.exceptions.InvalidRequestException; public class UFTest extends CQLTester { - public static Double sin(Double val) - { - return val != null ? Math.sin(val) : null; - } - - public static Float sin(Float val) - { - return val != null ? (float)Math.sin(val) : null; - } - - public static Double badSin(Double val) - { - return 42.0; - } - - public static String badSinBadReturn(Double val) - { - return "foo"; - } - - public Float nonStaticMethod(Float val) - { - return new Float(1.0); - } - - private static Float privateMethod(Float val) - { - return new Float(1.0); - } - - public static String repeat(String v, Integer n) - { - StringBuilder sb = new StringBuilder(); - for (int i = 0; i < n; i++) - sb.append(v); - return sb.toString(); - } - - public static String overloaded(String v) - { - return "f1"; - } - - public static String overloaded(Integer v) - { - return "f2"; - } - - public static String overloaded(String v1, String v2) - { - return "f3"; - } - @Test public void testFunctionCreationAndDrop() throws Throwable { @@ -89,26 +36,12 @@ public class UFTest extends CQLTester execute("INSERT INTO %s(key, d) VALUES (?, ?)", 2, 2d); execute("INSERT INTO %s(key, d) VALUES (?, ?)", 3, 3d); - // creation with a bad class - assertInvalid("CREATE FUNCTION foo::sin1 ( input double ) RETURNS double USING 'org.apache.cassandra.cql3.DoesNotExist#doesnotexist'"); - // and a good class but inexisting method - assertInvalid("CREATE FUNCTION foo::sin2 ( input double ) RETURNS double USING 'org.apache.cassandra.cql3.UFTest#doesnotexist'"); - // with a non static method - assertInvalid("CREATE FUNCTION foo::sin3 ( input float ) RETURNS float USING 'org.apache.cassandra.cql3.UFTest#nonStaticMethod'"); - // with a non public method - assertInvalid("CREATE FUNCTION foo::sin4 ( input float ) RETURNS float USING 'org.apache.cassandra.cql3.UFTest#privateMethod'"); - - // creation with bad argument types - assertInvalid("CREATE FUNCTION foo::sin5 ( input text ) RETURNS double USING 'org.apache.cassandra.cql3.UFTest#sin'"); - // with bad return types - assertInvalid("CREATE FUNCTION foo::sin6 ( input double ) RETURNS text USING 'org.apache.cassandra.cql3.UFTest#sin'"); - // simple creation - execute("CREATE FUNCTION foo::sin ( input double ) RETURNS double USING 'org.apache.cassandra.cql3.UFTest#sin'"); + execute("CREATE FUNCTION foo::sin ( input double ) RETURNS double LANGUAGE java AS 'return Double.valueOf(Math.sin(input.doubleValue()));'"); // check we can't recreate the same function - assertInvalid("CREATE FUNCTION foo::sin ( input double ) RETURNS double USING 'org.apache.cassandra.cql3.UFTest#sin'"); + assertInvalid("CREATE FUNCTION foo::sin ( input double ) RETURNS double LANGUAGE java AS 'return Double.valueOf(Math.sin(input.doubleValue()));'"); // but that it doesn't complay with "IF NOT EXISTS" - execute("CREATE FUNCTION IF NOT EXISTS foo::sin ( input double ) RETURNS double USING 'org.apache.cassandra.cql3.UFTest#sin'"); + execute("CREATE FUNCTION IF NOT EXISTS foo::sin ( input double ) RETURNS double LANGUAGE java AS 'return Double.valueOf(Math.sin(input.doubleValue()));'"); // Validate that it works as expected assertRows(execute("SELECT key, foo::sin(d) FROM %s"), @@ -117,10 +50,8 @@ public class UFTest extends CQLTester row(3, Math.sin(3d)) ); - // Replace the method with incompatible return type - assertInvalid("CREATE OR REPLACE FUNCTION foo::sin ( input double ) RETURNS text USING 'org.apache.cassandra.cql3.UFTest#badSinBadReturn'"); // proper replacement - execute("CREATE OR REPLACE FUNCTION foo::sin ( input double ) RETURNS double USING 'org.apache.cassandra.cql3.UFTest#badSin'"); + execute("CREATE OR REPLACE FUNCTION foo::sin ( input double ) RETURNS double LANGUAGE java AS 'return Double.valueOf(42d);'"); // Validate the method as been replaced assertRows(execute("SELECT key, foo::sin(d) FROM %s"), @@ -130,7 +61,7 @@ public class UFTest extends CQLTester ); // same function but without namespace - execute("CREATE FUNCTION sin ( input double ) RETURNS double USING 'org.apache.cassandra.cql3.UFTest#sin'"); + execute("CREATE FUNCTION sin ( input double ) RETURNS double LANGUAGE java AS 'return Double.valueOf(Math.sin(input.doubleValue()));'"); assertRows(execute("SELECT key, sin(d) FROM %s"), row(1, Math.sin(1d)), row(2, Math.sin(2d)), @@ -161,7 +92,10 @@ public class UFTest extends CQLTester execute("INSERT INTO %s(v) VALUES (?)", "aaa"); - execute("CREATE FUNCTION repeat (v text, n int) RETURNS text USING 'org.apache.cassandra.cql3.UFTest#repeat'"); + execute("CREATE FUNCTION repeat (v text, n int) RETURNS text LANGUAGE java AS 'StringBuilder sb = new StringBuilder();\n" + + " for (int i = 0; i < n.intValue(); i++)\n" + + " sb.append(v);\n" + + " return sb.toString();'"); assertRows(execute("SELECT v FROM %s WHERE v=repeat(?, ?)", "a", 3), row("aaa")); assertEmpty(execute("SELECT v FROM %s WHERE v=repeat(?, ?)", "a", 2)); @@ -174,13 +108,13 @@ public class UFTest extends CQLTester execute("INSERT INTO %s(k, v) VALUES (?, ?)", "f2", 1); - execute("CREATE FUNCTION overloaded(v varchar) RETURNS text USING 'org.apache.cassandra.cql3.UFTest'"); - execute("CREATE OR REPLACE FUNCTION overloaded(i int) RETURNS text USING 'org.apache.cassandra.cql3.UFTest'"); - execute("CREATE OR REPLACE FUNCTION overloaded(v1 text, v2 text) RETURNS text USING 'org.apache.cassandra.cql3.UFTest'"); - execute("CREATE OR REPLACE FUNCTION overloaded(v ascii) RETURNS text USING 'org.apache.cassandra.cql3.UFTest'"); + execute("CREATE FUNCTION overloaded(v varchar) RETURNS text LANGUAGE java AS 'return \"f1\";'"); + execute("CREATE OR REPLACE FUNCTION overloaded(i int) RETURNS text LANGUAGE java AS 'return \"f2\";'"); + execute("CREATE OR REPLACE FUNCTION overloaded(v1 text, v2 text) RETURNS text LANGUAGE java AS 'return \"f3\";'"); + execute("CREATE OR REPLACE FUNCTION overloaded(v ascii) RETURNS text LANGUAGE java AS 'return \"f1\";'"); // text == varchar, so this should be considered as a duplicate - assertInvalid("CREATE FUNCTION overloaded(v varchar) RETURNS text USING 'org.apache.cassandra.cql3.UFTest'"); + assertInvalid("CREATE FUNCTION overloaded(v varchar) RETURNS text LANGUAGE java AS 'return \"f1\";'"); assertRows(execute("SELECT overloaded(k), overloaded(v), overloaded(k, k) FROM %s"), row("f1", "f2", "f3") @@ -445,7 +379,7 @@ public class UFTest extends CQLTester execute("create function foo::pgfun1 ( input double ) returns text language java\n" + "AS $$" + functionBody + "$$;"); - execute("CREATE FUNCTION foo::pgsin ( input double ) RETURNS double USING $$org.apache.cassandra.cql3.UFTest#sin$$"); + execute("CREATE FUNCTION foo::pgsin ( input double ) RETURNS double LANGUAGE java AS $$return Double.valueOf(Math.sin(input.doubleValue()));$$"); assertRows(execute("SELECT language, body FROM system.schema_functions WHERE namespace='foo' AND name='pgfun1'"), row("java", functionBody)); @@ -600,7 +534,7 @@ public class UFTest extends CQLTester execute("CREATE OR REPLACE FUNCTION js(val double) RETURNS decimal LANGUAGE javascript\n" + "AS '100;';"); assertRows(execute("SELECT key, val, js(val) FROM %s"), - row(1, 1d, BigDecimal.valueOf(100d))); + row(1, 1d, BigDecimal.valueOf(100L))); execute("DROP FUNCTION js(double)"); // declared rtype = decimal , return type = double