diff --git a/src/Functions/FunctionFactory.cpp b/src/Functions/FunctionFactory.cpp index fe737740104..64e5dce413d 100644 --- a/src/Functions/FunctionFactory.cpp +++ b/src/Functions/FunctionFactory.cpp @@ -15,8 +15,6 @@ namespace DB { -class UserDefinedFunction; - namespace ErrorCodes { extern const int UNKNOWN_FUNCTION; diff --git a/src/Functions/FunctionFactory.h b/src/Functions/FunctionFactory.h index ea1ccc21eb9..bac55dade16 100644 --- a/src/Functions/FunctionFactory.h +++ b/src/Functions/FunctionFactory.h @@ -4,11 +4,9 @@ #include #include #include -#include #include #include -#include #include #include @@ -42,10 +40,6 @@ public: registerFunction(name, &Function::create, case_sensitiveness); } - void registerUserDefinedFunction(const ASTCreateFunctionQuery & create_function_query); - - void unregisterUserDefinedFunction(const String & function_name); - /// This function is used by YQL - internal Yandex product that depends on ClickHouse by source code. std::vector getAllNames() const; diff --git a/src/Interpreters/InterpreterCreateFunctionQuery.cpp b/src/Interpreters/InterpreterCreateFunctionQuery.cpp index 0e3e01ed8b9..577df6fc2b3 100644 --- a/src/Interpreters/InterpreterCreateFunctionQuery.cpp +++ b/src/Interpreters/InterpreterCreateFunctionQuery.cpp @@ -1,4 +1,6 @@ #include +#include +#include #include #include #include @@ -6,7 +8,7 @@ #include #include #include -#include + namespace DB { @@ -20,7 +22,9 @@ namespace ErrorCodes BlockIO InterpreterCreateFunctionQuery::execute() { - getContext()->checkAccess(AccessType::CREATE_FUNCTION); + auto context = getContext(); + context->checkAccess(AccessType::CREATE_FUNCTION); + FunctionNameNormalizer().visit(query_ptr.get()); auto * create_function_query = query_ptr->as(); @@ -34,15 +38,14 @@ BlockIO InterpreterCreateFunctionQuery::execute() if (!is_internal) { - try { - UserDefinedObjectsLoader::instance().storeObject(getContext(), UserDefinedObjectType::Function, function_name, *query_ptr); + UserDefinedObjectsLoader::instance().storeObject(context, UserDefinedObjectType::Function, function_name, *query_ptr); } - catch (Exception & e) + catch (Exception & exception) { UserDefinedFunctionFactory::instance().unregisterFunction(function_name); - e.addMessage(fmt::format("while storing user defined function {} on disk", backQuote(function_name))); + exception.addMessage(fmt::format("while storing user defined function {} on disk", backQuote(function_name))); throw; } } diff --git a/src/Interpreters/InterpreterCreateFunctionQuery.h b/src/Interpreters/InterpreterCreateFunctionQuery.h index e2ec09f9842..b10760c5e9d 100644 --- a/src/Interpreters/InterpreterCreateFunctionQuery.h +++ b/src/Interpreters/InterpreterCreateFunctionQuery.h @@ -1,12 +1,11 @@ #pragma once #include -#include + namespace DB { -class ASTCreateFunctionQuery; class Context; class InterpreterCreateFunctionQuery : public IInterpreter, WithContext diff --git a/src/Interpreters/InterpreterDropFunctionQuery.cpp b/src/Interpreters/InterpreterDropFunctionQuery.cpp index 13d932b9f7f..de27065ed83 100644 --- a/src/Interpreters/InterpreterDropFunctionQuery.cpp +++ b/src/Interpreters/InterpreterDropFunctionQuery.cpp @@ -12,11 +12,14 @@ namespace DB BlockIO InterpreterDropFunctionQuery::execute() { - getContext()->checkAccess(AccessType::DROP_FUNCTION); + auto context = getContext(); + context->checkAccess(AccessType::DROP_FUNCTION); + FunctionNameNormalizer().visit(query_ptr.get()); auto & drop_function_query = query_ptr->as(); + UserDefinedFunctionFactory::instance().unregisterFunction(drop_function_query.function_name); - UserDefinedObjectsLoader::instance().removeObject(getContext(), UserDefinedObjectType::Function, drop_function_query.function_name); + UserDefinedObjectsLoader::instance().removeObject(context, UserDefinedObjectType::Function, drop_function_query.function_name); return {}; } diff --git a/src/Interpreters/InterpreterDropFunctionQuery.h b/src/Interpreters/InterpreterDropFunctionQuery.h index 31d914948ed..5842851f5db 100644 --- a/src/Interpreters/InterpreterDropFunctionQuery.h +++ b/src/Interpreters/InterpreterDropFunctionQuery.h @@ -5,7 +5,6 @@ namespace DB { -class ASTDropFunctionQuery; class Context; class InterpreterDropFunctionQuery : public IInterpreter, WithMutableContext diff --git a/src/Interpreters/InterpreterFactory.cpp b/src/Interpreters/InterpreterFactory.cpp index 49de5b3049d..7061842a67f 100644 --- a/src/Interpreters/InterpreterFactory.cpp +++ b/src/Interpreters/InterpreterFactory.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include diff --git a/src/Interpreters/UserDefinedFunctionFactory.cpp b/src/Interpreters/UserDefinedFunctionFactory.cpp index c0fba635ce6..5cafb764f37 100644 --- a/src/Interpreters/UserDefinedFunctionFactory.cpp +++ b/src/Interpreters/UserDefinedFunctionFactory.cpp @@ -40,12 +40,13 @@ void UserDefinedFunctionFactory::unregisterFunction(const String & function_name AggregateFunctionFactory::instance().hasNameOrAlias(function_name)) throw Exception(ErrorCodes::CANNOT_DROP_SYSTEM_FUNCTION, "Cannot drop system function '{}'", function_name); - auto it = function_name_to_create_query.find(function_name); if (it == function_name_to_create_query.end()) throw Exception(ErrorCodes::UNKNOWN_FUNCTION, "The function name '{}' is not registered", function_name); + + function_name_to_create_query.erase(it); } ASTPtr UserDefinedFunctionFactory::get(const String & function_name) const diff --git a/tests/integration/test_access_for_functions/test.py b/tests/integration/test_access_for_functions/test.py index 6eacd906079..ebd0f6bd907 100644 --- a/tests/integration/test_access_for_functions/test.py +++ b/tests/integration/test_access_for_functions/test.py @@ -14,16 +14,6 @@ def started_cluster(): finally: cluster.shutdown() - -@pytest.fixture(autouse=True) -def cleanup_after_test(): - try: - yield - finally: - instance.query("DROP USER IF EXISTS A") - instance.query("DROP USER IF EXISTS B") - - def test_access_rights_for_funtion(): create_function_query = "CREATE FUNCTION MySum AS (a, b) -> a + b" @@ -32,7 +22,7 @@ def test_access_rights_for_funtion(): assert "it's necessary to have grant CREATE FUNCTION ON *.*" in instance.query_and_get_error(create_function_query, user = 'A') instance.query("GRANT CREATE FUNCTION on *.* TO A") - + instance.query(create_function_query, user = 'A') assert instance.query("SELECT MySum(1, 2)") == "3\n" @@ -44,3 +34,6 @@ def test_access_rights_for_funtion(): instance.query("REVOKE CREATE FUNCTION ON *.* FROM A") assert "it's necessary to have grant CREATE FUNCTION ON *.*" in instance.query_and_get_error(create_function_query, user = 'A') + + instance.query("DROP USER IF EXISTS A") + instance.query("DROP USER IF EXISTS B") diff --git a/tests/queries/0_stateless/01855_create_simple_function.sql b/tests/queries/0_stateless/01855_create_simple_function.sql deleted file mode 100644 index 36e3f8cfd94..00000000000 --- a/tests/queries/0_stateless/01855_create_simple_function.sql +++ /dev/null @@ -1,4 +0,0 @@ -create function MyFunc as (a, b, c) -> a * b * c; -select MyFunc(2, 3, 4); -select isConstant(MyFunc(1, 2, 3)); -drop function MyFunc; diff --git a/tests/queries/0_stateless/01855_create_simple_function.reference b/tests/queries/0_stateless/01856_create_function.reference similarity index 100% rename from tests/queries/0_stateless/01855_create_simple_function.reference rename to tests/queries/0_stateless/01856_create_function.reference diff --git a/tests/queries/0_stateless/01856_create_function.sql b/tests/queries/0_stateless/01856_create_function.sql new file mode 100644 index 00000000000..7e2f38c2415 --- /dev/null +++ b/tests/queries/0_stateless/01856_create_function.sql @@ -0,0 +1,13 @@ +CREATE FUNCTION 01856_test_function_0 AS (a, b, c) -> a * b * c; +SELECT 01856_test_function_0(2, 3, 4); +SELECT isConstant(01856_test_function_0(1, 2, 3)); +DROP FUNCTION 01856_test_function_0; +CREATE FUNCTION 01856_test_function_1 AS (a, b) -> a || b || c; --{serverError 47} +CREATE FUNCTION 01856_test_function_1 AS (a, b) -> 01856_test_function_1(a, b) + 01856_test_function_1(a, b); --{serverError 600} +CREATE FUNCTION cast AS a -> a + 1; --{serverError 598} +CREATE FUNCTION sum AS (a, b) -> a + b; --{serverError 598} +CREATE FUNCTION 01856_test_function_2 AS (a, b) -> a + b; +CREATE FUNCTION 01856_test_function_2 AS (a) -> a || '!!!'; --{serverError 598} +DROP FUNCTION 01856_test_function_2; +DROP FUNCTION unknown_function; -- {serverError 46} +DROP FUNCTION CAST; -- {serverError 599} diff --git a/tests/queries/0_stateless/01856_create_function_errors.reference b/tests/queries/0_stateless/01856_create_function_errors.reference deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/tests/queries/0_stateless/01856_create_function_errors.sql b/tests/queries/0_stateless/01856_create_function_errors.sql deleted file mode 100644 index 9da8f8e9822..00000000000 --- a/tests/queries/0_stateless/01856_create_function_errors.sql +++ /dev/null @@ -1,17 +0,0 @@ --- CREATE FUNCTION MyFunc2 AS (a, b) -> a || b || c; --{serverError 47} - --- CREATE FUNCTION MyFunc2 AS (a, b) -> MyFunc2(a, b) + MyFunc2(a, b); --{serverError 600} - --- CREATE FUNCTION cast AS a -> a + 1; --{serverError 598} - --- CREATE FUNCTION sum AS (a, b) -> a + b; --{serverError 598} - --- CREATE FUNCTION MyFunc3 AS (a, b) -> a + b; - --- CREATE FUNCTION MyFunc3 AS (a) -> a || '!!!'; --{serverError 598} - --- DROP FUNCTION MyFunc3; - --- DROP FUNCTION unknownFunc; -- {serverError 46} - -DROP FUNCTION CAST; -- {serverError 599} diff --git a/tests/queries/skip_list.json b/tests/queries/skip_list.json index 065ba8bc593..0aa4dc59098 100644 --- a/tests/queries/skip_list.json +++ b/tests/queries/skip_list.json @@ -483,8 +483,7 @@ "01804_dictionary_decimal256_type", "01850_dist_INSERT_preserve_error", // uses cluster with different static databases shard_0/shard_1 "01821_table_comment", - "01855_create_simple_function", - "01856_create_function_errors", + "01856_create_function", "01857_create_function_and_check_jit_compiled", "01824_prefer_global_in_and_join", "01870_modulo_partition_key",