maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Alexey,

On Mar 02, Alexey Botchkov wrote:
> revision-id: 9d6be81c0da (mariadb-10.7.2-8-g9d6be81c0da)
> parent(s): 33fd136c61b
> author: Alexey Botchkov
> committer: Alexey Botchkov
> timestamp: 2022-02-15 13:37:59 +0400
> message:
> 
> MDEV-27832 disable binary logging for SQL SERVICE.
> 
> Binary logging is now disabled for the queries run by SQL SERVICE.
> The binlogging can be turned on with the 'SET SQL_LOG_BIN=On' query.
> 
> diff --git a/mysql-test/suite/plugins/r/test_sql_service.result b/mysql-test/suite/plugins/r/test_sql_service.result
> index 00f0411b665..943bb239e5f 100644
> --- a/mysql-test/suite/plugins/r/test_sql_service.result
> +++ b/mysql-test/suite/plugins/r/test_sql_service.result
> @@ -2,6 +2,19 @@ install plugin test_sql_service soname 'test_sql_service';
>  show status like 'test_sql_service_passed';
>  Variable_name	Value
>  Test_sql_service_passed	1
> +set global test_sql_service_execute_sql_global= 'create table test.t1 select 1 as a, @@SQL_LOG_BIN';
> +set global test_sql_service_execute_sql_local=  'insert into test.t1 select 2 as a, @@SQL_LOG_BIN';
> +set global test_sql_service_execute_sql_global= 'SET SQL_LOG_BIN=1';
> +set global test_sql_service_execute_sql_global= 'insert into test.t1 select 3 as a, @@SQL_LOG_BIN';
> +set global test_sql_service_execute_sql_global= 'SET SQL_LOG_BIN=0';
> +set global test_sql_service_execute_sql_global= 'insert into test.t1 select 4 as a, @@SQL_LOG_BIN';
> +select * from t1 order by a;

this is not a test. Please, enable binlog and see whether queries are
actually logged.

Also, add a test for, say, sql_auto_is_null:

  set global test_sql_service_execute_sql_global='set sql_auto_is_null=1';
  set global test_sql_service_execute_sql_global='insert test.t1 select 5, @@sql_auto_is_null';
  select * from t1 where a=5;

> +a	@@SQL_LOG_BIN
> +1	0
> +2	0
> +3	1
> +4	0
> +drop table t1;
>  set global test_sql_service_run_test= 1;
>  show status like 'test_sql_service_passed';
>  Variable_name	Value
> diff --git a/sql/sql_prepare.cc b/sql/sql_prepare.cc
> index e025147c71e..48681ac0b6f 100644
> --- a/sql/sql_prepare.cc
> +++ b/sql/sql_prepare.cc
> @@ -5617,12 +5617,30 @@ class Protocol_local : public Protocol_text
>    THD *new_thd;
>    Security_context empty_ctx;
>  
> +  my_bool do_log_bin;
> +
>    Protocol_local(THD *thd_arg, THD *new_thd_arg, ulong prealloc) :
>      Protocol_text(thd_arg, prealloc),
>      cur_data(0), first_data(0), data_tail(&first_data), alloc(0),
> -    new_thd(new_thd_arg)
> +    new_thd(new_thd_arg), do_log_bin(FALSE)
>    {}
>   
> +  void set_binlog_vars(my_bool *sav_log_bin, ulonglong *sav_bits)
> +  {
> +    *sav_log_bin= thd->variables.sql_log_bin;
> +    *sav_bits= thd->variables.option_bits;
> +
> +    if ((thd->variables.sql_log_bin= do_log_bin))
> +      thd->variables.option_bits|= OPTION_BIN_LOG;
> +    else
> +      thd->variables.option_bits&= ~OPTION_BIN_LOG;
> +  }
> +  void restore_binlog_vars(my_bool &sav_log_bin, ulonglong &sav_bits)
> +  {
> +    do_log_bin= thd->variables.sql_log_bin;
> +    thd->variables.sql_log_bin= sav_log_bin;
> +    thd->variables.option_bits= sav_bits;

that's what sql_auto_is_null test is for.You destroy all other option_bits
here. Better don't save/restore option_bits, but use the same if() as in
set_binlog_vars(). Or, even better, reuse
fix_sql_log_bin_after_update(). I'd suggest to make it a method in THD

> +  }
>  protected:
>    bool net_store_data(const uchar *from, size_t length);
>    bool net_store_data_cs(const uchar *from, size_t length,
> @@ -6230,12 +6248,17 @@ loc_advanced_command(MYSQL *mysql, enum enum_server_command command,
>      Ed_connection con(p->thd);
>      Security_context *ctx_orig= p->thd->security_ctx;
>      MYSQL_LEX_STRING sql_text;
> +    my_bool log_bin_orig;
> +    ulonglong option_bits_orig;
> +    p->set_binlog_vars(&log_bin_orig, &option_bits_orig);
> +
>      DBUG_ASSERT(current_thd == p->thd);
>      sql_text.str= (char *) arg;
>      sql_text.length= arg_length;
>      p->thd->security_ctx= &p->empty_ctx;
>      result= con.execute_direct(p, sql_text);
>      p->thd->security_ctx= ctx_orig;
> +    p->restore_binlog_vars(log_bin_orig, option_bits_orig);
>    }
>    if (skip_check)
>      result= 0;
> @@ -6391,6 +6414,9 @@ extern "C" MYSQL *mysql_real_connect_local(MYSQL *mysql)
>      new_thd->security_ctx->skip_grants();
>      new_thd->query_cache_is_applicable= 0;
>      new_thd->variables.wsrep_on= 0;
> +    new_thd->variables.sql_log_bin= 0;
> +    new_thd->variables.option_bits&= ~OPTION_BIN_LOG;
> +             
>      /*
>        TOSO: decide if we should turn the auditing off
>        for such threads.
 
Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx