maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Alexey!

On Sep 10, Alexey Botchkov wrote:
> > For example, I'd think it connects to the current server, internally.
> > But then, why does it need host and user?
> 
> Initially i thought it can be put in the thread's security_ctx so functions
> like CURRENT_USER
> return these values. After some more meditation i decided that it
> doesn't seem to be useful and just removed these arguments.

What security context are you using now?
root? whatever happen to be in the THD?
What if you create a new THD?

> > > +ADD_DEFINITIONS(-DMYSQL_SERVER)
> > Why?
> Couldn't figure out how to get rid of it.
> Well your trick with the '#if !defined(MYSQL_SERVICE_SQL)' in the mysql.h
> helps.
> I wasn't bold enough to mention the particular service in such an exposed
> file as mysql.h :)

Feel free to replace it with any other macro that is guaranteed to be
set for the server code (including plugins and anything that doesn't
define MYSQL_SERVER, but still is in the server). Any plugin related
define should work, I think.

> > > +#include <mysql.h>
> > shouldn't be needed
> My idea was that those not using the SQL service don't have to see the
> 'mysql.h' declarations.
> Now all the plugins see all these mysql_xxx() functions and related
> structures. Not sure if it's good.

I simply mean that you already include mysql.h in the service_sql.h.

If you wouldn't, then yes, a separate include would be needed.

> The update pushed 71338ddada9f331af57b737a586f56dc6f2e483f.

Thanks. One unanswered question below:

> diff --git a/sql/sql_class.h b/sql/sql_class.h
> index e569fcd32d6..964626be3d4 100644
> --- a/sql/sql_class.h
> +++ b/sql/sql_class.h
> diff --git a/sql/sql_prepare.cc b/sql/sql_prepare.cc
> index 09ad632dd98..4725855c130 100644
> --- a/sql/sql_prepare.cc
> +++ b/sql/sql_prepare.cc
> @@ -4857,6 +4870,7 @@ Prepared_statement::execute_server_runnable(Server_runnable *server_runnable)
>    if (!(lex= new (mem_root) st_lex_local))
>      return TRUE;
>
> +  thd->set_time();

why? May be it should be executed using the timestamp of the top-level
statement?

>    thd->set_n_backup_statement(this, &stmt_backup);
>    thd->set_n_backup_active_arena(this, &stmt_backup);
>    thd->stmt_arena= this;

Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx