|
http://lists.askmonty.org/pipermail/commits/2019-April/013658.html
|
|
I'll comment here, I had some issues with email and your commit email was apparently lost.
The main thought — in this particular case using a service (as in libservice/HOWTO) looks somewhat artificial. A service is a way to create a stable external API out of server internals for plugins to use. But here there is already an API - client-server API, and you build on it (which is great). I'm not sure it adds a lot of value to create a service to proxy another external API, perhaps a plugin can simply use <mysql.h> without requiring a service wrapper?
A couple of smaller comments:
> diff --git a/sql-common/client.c b/sql-common/client.c
|
> index c66cb1a..ace5630 100644
|
> --- a/sql-common/client.c
|
> +++ b/sql-common/client.c
|
> @@ -2844,6 +2844,11 @@ set_connect_attributes(MYSQL *mysql, char *buff, size_t buf_len)
|
> }
|
>
|
>
|
> +#ifdef MYSQL_SERVER
|
> +extern MYSQL * STDCALL do_local_connect(MYSQL *mysql);
|
> +#endif /*MYSQL_SERVER*/
|
|
this shouldn't need an #ifdef, one ifdef below is enough.
|
|
alternatively, you can remove ifdef below and do here
|
|
#ifdef MYSQL_SERVER
|
#define do_local_connect(X) 1
|
#endif
|
|
this is, probably, the preferrable way, keeps ifdefs out of the code.
|
|
> +
|
> +
|
> MYSQL * STDCALL
|
> CLI_MYSQL_REAL_CONNECT(MYSQL *mysql,const char *host, const char *user,
|
> const char *passwd, const char *db,
|
> diff --git a/sql/service_sql.cc b/sql/service_sql.cc
|
> new file mode 100644
|
> index 0000000..36f65b7
|
> --- /dev/null
|
> +++ b/sql/service_sql.cc
|
> @@ -0,0 +1,43 @@
|
> +/* Copyright 2018 Codership Oy
|
|
Is that right?
|
|
> +
|
> + 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.
|
|
|
for now the plan is to reshpe the patch so it's possible to just include mysql.h to get the interface to the local connection since the server includes the libmysql in it.
Then we should move pieces of the embedded server into the normal server that provide local data transfer, and make loc_... methods with them.
|
|
mysql_real_escape_string present in headers but has nothing to link with
|
|
if (mysql_real_query(mysql,
|
STRING_WITH_LEN("CREATE TABLE mysql." HISTORY_DB_NAME
|
" ( hash varbinary(512),"
|
" time timestamp default current_time,"
|
" primary key (hash), index tm (time) )")))
|
{
|
crashes the server on attempt to insert with default time
and such table:
if (mysql_real_query(mysql,
|
STRING_WITH_LEN("CREATE TABLE mysql." HISTORY_DB_NAME
|
" ( hash varbinary(512),"
|
" time timestamp default current_time,"
|
" primary key (hash), index tm (time) )")))
|
{
|
inserts 0000-00-00 00:00:00 but according to table definition shoud include current time:
show create table mysql.reuse_password_check_history;
|
Table Create Table
|
reuse_password_check_history CREATE TABLE `reuse_password_check_history` (
|
`hash` varbinary(512) NOT NULL,
|
`time` timestamp NOT NULL DEFAULT current_timestamp() ON UPDATE current_timestamp(),
|
PRIMARY KEY (`hash`),
|
KEY `tm` (`time`)
|
) ENGINE=MyISAM DEFAULT CHARSET=latin1
|
|
|
the implementation proposal for today.
https://github.com/mariadb/server/commit/b638b0f515976327c2b11b730747931369101990
|
|
This does not built:
FAILED: sql/CMakeFiles/sql.dir/sql_plugin.cc.o
|
/usr/bin/c++ -DDBUG_TRACE -DHAVE_CONFIG_H -DHAVE_EVENT_SCHEDULER -DHAVE_OPENSSL -DHAVE_POOL_OF_THREADS -DMYSQL_SERVER -D_FILE_OFFSET_BITS=64 -Iwsrep-lib/include -Iwsrep-lib/wsrep-API/v26 -Iinclude -Isql -Iextra/pcre2/src/pcre2-build -Iextra/pcre2/src/pcre2/src -Itpool -fPIC -g -DENABLED_DEBUG_SYNC -ggdb3 -DSAFE_MUTEX -DSAFEMALLOC -DTRASH_FREED_MEMORY -Wall -Wextra -Wformat-security -Wno-format-truncation -Wno-init-self -Wno-nonnull-compare -Wno-unused-parameter -Woverloaded-virtual -Wnon-virtual-dtor -Wvla -Wwrite-strings -Werror -std=gnu++11 -MD -MT sql/CMakeFiles/sql.dir/sql_plugin.cc.o -MF sql/CMakeFiles/sql.dir/sql_plugin.cc.o.d -o sql/CMakeFiles/sql.dir/sql_plugin.cc.o -c sql/sql_plugin.cc
|
sql/sql_plugin.cc: In member function ‘virtual bool sys_var_pluginvar::global_update(THD*, set_var*)’:
|
sql/sql_plugin.cc:3622:27: error: ‘PLUGIN_VAR_NO_LOCK’ was not declared in this scope; did you mean ‘PLUGIN_VAR_LONGLONG’?
|
3622 | if (plugin_var->flags & PLUGIN_VAR_NO_LOCK)
|
| ^~~~~~~~~~~~~~~~~~
|
| PLUGIN_VAR_LONGLONG
|
sql/sql_plugin.cc:3627:27: error: ‘PLUGIN_VAR_NO_LOCK’ was not declared in this scope; did you mean ‘PLUGIN_VAR_LONGLONG’?
|
3627 | if (plugin_var->flags & PLUGIN_VAR_NO_LOCK)
|
| ^~~~~~~~~~~~~~~~~~
|
| PLUGIN_VAR_LONGLONG
|
[21/106] Building CXX object sql/CMakeFiles/sql.dir/uniques.cc.o
|
ninja: build stopped: subcommand failed.
|
|
|
Problem with timestamp is not solved (branch to check bb-10.7-MDEV-9245-3)
|
|
Proposed patch
https://github.com/mariadb/server/commit/eac481629b8e7b85e2ef862198c9f3eab2dd0a47
|