Hi, Alexander! Just a couple of comments, see below: On Oct 16, Alexander Barkov wrote:

revision-id: a0e3bd09251 (mariadb-10.4.4-419-ga0e3bd09251) parent(s): 22b645ef529 author: Alexander Barkov <bar@mariadb.com> committer: Alexander Barkov <bar@mariadb.com> timestamp: 2019-10-16 16:26:29 +0400 message:

Part1: MDEV-20837 Add MariaDB_FUNCTION_PLUGIN

- Defining MariaDB_FUNCTION_PLUGIN - Changing the code in /plugins/type_inet/ and /plugins/type_test/ to use MariaDB_FUNCTION_PLUGIN instead of MariaDB_FUNCTION_COLLECTION_PLUGIN.

diff --git a/include/mysql/plugin_function.h b/include/mysql/plugin_function.h new file mode 100644 index 00000000000..4ce416612e9 --- /dev/null +++ b/include/mysql/plugin_function.h @@ -0,0 +1,58 @@ +#ifndef MARIADB_PLUGIN_FUNCTION_INCLUDED +#define MARIADB_PLUGIN_FUNCTION_INCLUDED +/* Copyright (C) 2019, Alexander Barkov and MariaDB + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; version 2 of the License. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, write to the Free Software + Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1335 USA */ + +/** + @file + + Function Plugin API. + + This file defines the API for server plugins that manage functions. +*/ + +#ifdef __cplusplus + +#include <mysql/plugin.h> + +/* + API for function plugins. (MariaDB_FUNCTION_PLUGIN) +*/ +#define MariaDB_FUNCTION_INTERFACE_VERSION (MYSQL_VERSION_ID << 8) + + +class Plugin_function +{ + int m_interface_version; + Create_func *m_builder; +public: + Plugin_function(int interface_version, Create_func *builder) + :m_interface_version(interface_version), + m_builder(builder) + { }

Why do you need this ^^^ constructor?

+ Plugin_function(Create_func *builder) + :m_interface_version(MariaDB_FUNCTION_INTERFACE_VERSION), + m_builder(builder) + { } + Create_func *create_func() + { + return m_builder; + } +}; + + +#endif /* __cplusplus */ + +#endif /* MARIADB_PLUGIN_FUNCTION_INCLUDED */ diff --git a/plugin/type_inet/mysql-test/type_inet/func_inet_plugin.result b/plugin/type_inet/mysql-test/type_inet/func_inet_plugin.result index 4663ae485e2..a9422f2e4fd 100644 --- a/plugin/type_inet/mysql-test/type_inet/func_inet_plugin.result +++ b/plugin/type_inet/mysql-test/type_inet/func_inet_plugin.result @@ -15,14 +16,94 @@ PLUGIN_LICENSE, PLUGIN_MATURITY, PLUGIN_AUTH_VERSION FROM INFORMATION_SCHEMA.PLUGINS -WHERE PLUGIN_TYPE='FUNCTION COLLECTION' - AND PLUGIN_NAME='func_inet'; -PLUGIN_NAME func_inet +WHERE PLUGIN_TYPE='FUNCTION' + AND PLUGIN_NAME IN +('inet_aton', +'inet_ntoa', +'inet6_aton', +'inet6_ntoa', +'is_ipv4', +'is_ipv6', +'is_ipv4_compat', +'is_ipv4_mapped') +ORDER BY PLUGIN_NAME; +---- ---- +PLUGIN_NAME inet6_aton PLUGIN_VERSION 1.0 PLUGIN_STATUS ACTIVE -PLUGIN_TYPE FUNCTION COLLECTION +PLUGIN_TYPE FUNCTION PLUGIN_AUTHOR MariaDB Corporation -PLUGIN_DESCRIPTION Function collection test +PLUGIN_DESCRIPTION Function INET6_ATON() +PLUGIN_LICENSE GPL +PLUGIN_MATURITY Experimental

INET* functions should probably be Alpha, not Experimental. You expect them to be GA one day, don't you?

+PLUGIN_AUTH_VERSION 1.0 +---- ---- +PLUGIN_NAME inet6_ntoa +PLUGIN_VERSION 1.0 +PLUGIN_STATUS ACTIVE +PLUGIN_TYPE FUNCTION +PLUGIN_AUTHOR MariaDB Corporation +PLUGIN_DESCRIPTION Function INET6_NTOA() +PLUGIN_LICENSE GPL +PLUGIN_MATURITY Experimental +PLUGIN_AUTH_VERSION 1.0 +---- ---- +PLUGIN_NAME inet_aton +PLUGIN_VERSION 1.0 +PLUGIN_STATUS ACTIVE +PLUGIN_TYPE FUNCTION +PLUGIN_AUTHOR MariaDB Corporation +PLUGIN_DESCRIPTION Function INET_ATON() +PLUGIN_LICENSE GPL +PLUGIN_MATURITY Experimental +PLUGIN_AUTH_VERSION 1.0 +---- ---- +PLUGIN_NAME inet_ntoa +PLUGIN_VERSION 1.0 +PLUGIN_STATUS ACTIVE +PLUGIN_TYPE FUNCTION +PLUGIN_AUTHOR MariaDB Corporation +PLUGIN_DESCRIPTION Function INET_NTOA() +PLUGIN_LICENSE GPL +PLUGIN_MATURITY Experimental +PLUGIN_AUTH_VERSION 1.0 +---- ---- +PLUGIN_NAME is_ipv4 +PLUGIN_VERSION 1.0 +PLUGIN_STATUS ACTIVE +PLUGIN_TYPE FUNCTION +PLUGIN_AUTHOR MariaDB Corporation +PLUGIN_DESCRIPTION Function IS_IPV4() +PLUGIN_LICENSE GPL +PLUGIN_MATURITY Experimental +PLUGIN_AUTH_VERSION 1.0 +---- ---- +PLUGIN_NAME is_ipv4_compat +PLUGIN_VERSION 1.0 +PLUGIN_STATUS ACTIVE +PLUGIN_TYPE FUNCTION +PLUGIN_AUTHOR MariaDB Corporation +PLUGIN_DESCRIPTION Function IS_IPV4_COMPAT() +PLUGIN_LICENSE GPL +PLUGIN_MATURITY Experimental +PLUGIN_AUTH_VERSION 1.0 +---- ---- +PLUGIN_NAME is_ipv4_mapped +PLUGIN_VERSION 1.0 +PLUGIN_STATUS ACTIVE +PLUGIN_TYPE FUNCTION +PLUGIN_AUTHOR MariaDB Corporation +PLUGIN_DESCRIPTION Function IS_IPV4_MAPPED() +PLUGIN_LICENSE GPL +PLUGIN_MATURITY Experimental +PLUGIN_AUTH_VERSION 1.0 +---- ---- +PLUGIN_NAME is_ipv6 +PLUGIN_VERSION 1.0 +PLUGIN_STATUS ACTIVE +PLUGIN_TYPE FUNCTION +PLUGIN_AUTHOR MariaDB Corporation +PLUGIN_DESCRIPTION Function IS_IPV6() PLUGIN_LICENSE GPL PLUGIN_MATURITY Experimental PLUGIN_AUTH_VERSION 1.0 diff --git a/sql/item_create.cc b/sql/item_create.cc index e8eb76dfc12..e316723cedf 100644 --- a/sql/item_create.cc +++ b/sql/item_create.cc @@ -5704,12 +5657,26 @@ void item_create_cleanup() { DBUG_ENTER("item_create_cleanup"); my_hash_free(& native_functions_hash); -#ifdef HAVE_SPATIAL - plugin_function_collection_geometry.deinit(); -#endif DBUG_VOID_RETURN; }

+ +static Create_func * +function_plugin_find_native_function_builder(THD *thd, const LEX_CSTRING &name) +{ + plugin_ref plugin; + if ((plugin= my_plugin_lock_by_name(thd, &name, MariaDB_FUNCTION_PLUGIN))) + { + Create_func *builder= + reinterpret_cast<Plugin_function*>(plugin_decl(plugin)->info)-> + create_func(); + plugin_unlock(thd, plugin); + return builder; + } + return NULL;

So, function plugins cannot be unlocked either?

+} + + Create_func * find_native_function_builder(THD *thd, const LEX_CSTRING *name) { @@ -5724,16 +5691,10 @@ find_native_function_builder(THD *thd, const LEX_CSTRING *name) if (func && (builder= func->builder)) return builder;

- if ((builder= Plugin_find_native_func_builder_param(*name).find(thd))) + if ((builder= function_plugin_find_native_function_builder(thd, *name)))

this is what I mean by "special code path for plugins" and want to get rid of. But not in this commit, I agree.

return builder;

-#ifdef HAVE_SPATIAL - if (!builder) - builder= plugin_function_collection_geometry. - find_native_function_builder(thd, *name); -#endif - - return builder; + return NULL; }

Create_qfunc *

Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org