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 <b...@mariadb.com>
> committer: Alexander Barkov <b...@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 secur...@mariadb.org
_______________________________________________
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help : https://help.launchpad.net/ListHelp
