Re: [Maria-developers] 57bf586: MDEV-8615: Assertion `m_cpp_buf <= begin_ptr && begin_ptr <= m_cpp_buf + m_buf_length' failed in Lex_input_stream::body_utf8_start

2016-01-27 Thread Sergei Golubchik
Hi, Sanja!

On Jan 27, OleksandrByelkin wrote:
> revision-id: 57bf58668877552ee43f4ed86689b8bbb3821c9c 
> (mariadb-10.1.10-23-g57bf586)
> parent(s): 825f51d1aab51d363dc07ec9fe0829af33063883
> committer: Oleksandr Byelkin
> timestamp: 2016-01-27 00:38:36 +0100
> message:
> 
> MDEV-8615: Assertion `m_cpp_buf <= begin_ptr && begin_ptr <= m_cpp_buf + 
> m_buf_length' failed in Lex_input_stream::body_utf8_start
> 
> Nothing should be done before any keyword recognized.

Good!
Just two comments:

1. May be it's better to rename
  s/pop_cs_label/pop_sp_label/ and
  s/pop_cs_empty_label/pop_sp_empty_label/ ?
   first, pop_sp_empty_label applies to all SPs, not only to compound
   statements, and second, pop_cs_label does not apply to compound
   statements at all :)

2. Your process_control_statement_label() makes no sense here. There is
   no need to do an if() - because in both branches you always know
   whether you have a label or not. Please split
   process_control_statement_label() in two different functions
   (or two yacc rules, whatever).

> diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy
> index 302c2fd..3c2d386 100644
> --- a/sql/sql_yacc.yy
> +++ b/sql/sql_yacc.yy
> @@ -248,6 +248,36 @@ static bool maybe_start_compound_statement(THD *thd)
>return 0;
>  }
>  
> +static bool process_control_statement_label(THD *thd, LEX_STRING label)
> +{
> +  if (label.str != NULL)
> +  {
> +sp_pcontext *ctx= thd->lex->spcont;
> +sp_label *lab= ctx->find_label(label);
> +
> +if (lab)
> +{
> +  my_error(ER_SP_LABEL_REDEFINE, MYF(0), label.str);
> +  return 1;
> +}
> +else
> +{
> +  lab= thd->lex->spcont->push_label(thd, label,
> +thd->lex->sphead->instructions());
> +  lab->type= sp_label::ITERATION;
> +}
> +  }
> +  else
> +  {
> +if (maybe_start_compound_statement(thd))
> +  return 1;
> +/* Unlabeled controls get an empty label. */
> +thd->lex->spcont->push_label(thd, empty_lex_str,
> + thd->lex->sphead->instructions());
> +  }
> +  return 0;
> +}
> +
>  /**
>Helper action for a case expression statement (the expr in 'CASE expr').
>This helper is used for 'searched' cases only.
> @@ -4389,6 +4374,84 @@ sp_control_content:
>}
>  ;
>  
> +pop_cs_label:
> +  sp_opt_label
> +  {
> +sp_label *lab;
> +Lex->sphead->backpatch(lab= Lex->spcont->pop_label());
> +if ($1.str)
> +{
> +  if (my_strcasecmp(system_charset_info, $1.str,
> +lab->name.str) != 0)
> +  {
> +my_error(ER_SP_LABEL_MISMATCH, MYF(0), $1.str);
> +MYSQL_YYABORT;
> +  }
> +}
> +  }
> +;
> +
> +pop_cs_empty_label:
> +  {
> +sp_label *lab;
> +Lex->sphead->backpatch(lab= Lex->spcont->pop_label());
> +DBUG_ASSERT(lab->name.length == 0);
> +  }
> +;
> +
> +sp_labeled_control:
> +  label_ident ':' LOOP_SYM
> +  {
> +if (process_control_statement_label(thd, $1))
> +  MYSQL_YYABORT;
> +  }
> +  loop_body pop_cs_label
> +  { }
> +| label_ident ':' WHILE_SYM
> +  {
> +if (process_control_statement_label(thd, $1))
> +  MYSQL_YYABORT;
> +Lex->sphead->reset_lex(thd);
> +  }
> +  while_body pop_cs_label
> +  { }
> +| label_ident ':' REPEAT_SYM
> +  {
> +if (process_control_statement_label(thd, $1))
> +  MYSQL_YYABORT;
> +  }
> +  repeat_body pop_cs_label
> +  { }
> +;
> +
> +sp_unlabeled_control:
> +  LOOP_SYM
> +  {
> +if (process_control_statement_label(thd, null_lex_str))
> +  MYSQL_YYABORT;
> +  }
> +  loop_body
> +  pop_cs_empty_label
> +  { }
> +| WHILE_SYM
> +  {
> +if (process_control_statement_label(thd, null_lex_str))
> +  MYSQL_YYABORT;
> +Lex->sphead->reset_lex(thd);
> +  }
> +  while_body
> +  pop_cs_empty_label
> +  { }
> +| REPEAT_SYM
> +  {
> +if (process_control_statement_label(thd, null_lex_str))
> +  MYSQL_YYABORT;
> +  }
> +  repeat_body
> +  pop_cs_empty_label
> +  { }
> +;
> +
>  trg_action_time:
>  BEFORE_SYM
>  { Lex->trg_chistics.action_time= TRG_ACTION_BEFORE; }

Regards,
Sergei
Chief Architect MariaDB
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   : 

[Maria-developers] MDEV-5513 merge audit api changes from 5.5.34.

2016-01-27 Thread Alexey Botchkov

Hello Sergei, all.

I didn't quite understand what exact changes in 5.5.34 are important to us,
so in order to begin i'm going to mention shortly what is new in MySQL 
5.7 audit plugin API.


1. New event classes added:
MYSQL_AUDIT_PARSE_CLASS= 2,
  MYSQL_AUDIT_AUTHORIZATION_CLASS= 3,
  MYSQL_AUDIT_TABLE_ACCESS_CLASS = 4,
  MYSQL_AUDIT_GLOBAL_VARIABLE_CLASS  = 5,
  MYSQL_AUDIT_SERVER_STARTUP_CLASS   = 6,
  MYSQL_AUDIT_SERVER_SHUTDOWN_CLASS  = 7,
  MYSQL_AUDIT_COMMAND_CLASS  = 8,
  MYSQL_AUDIT_QUERY_CLASS= 9,
  MYSQL_AUDIT_STORED_PROGRAM_CLASS   = 10,
These new notifications are sent from various parts of the code. I think 
that part can be most

difficult to keep similar between Maria and My.

New data structures are declared for these classes to send data inside 
notify().
In my opinion these data structures only contain little information, not 
sufficient for many tasks.


2. Plugin can specify the CLASS/SUBCLASS filter in the st_mysql_audit 
structure.
   To implement that nubers of subclasses were changed (i mean 
previously known subclasses).


3. notify() function returns 'int' (it used to be 'void').

   I think 2. and 3. are the reasons MySQL doesn't load plugins with 
older interface.



4. Notification function can affect server.
- Nonzero return from the notify() breaks the query execution in 
some cases.

   - Query can be modified inside the 'notify' function in some cases.

5. Sending notification looks quite crazy in the code. All 
notification-sending functions share
the exactly same name 'mysql_audit_notify'. Naturally the number 
and types of parameters are different.
So that i have complications if i want to find all the places 
where  for instance the MYSQL_AUDIT_PARSE_CLASS

notifications are sent. 'grep' isn't much help anymore.

6. The code is immature. The MYSQL_AUDIT_TABLE_ACCESS_CLASS just is not 
supported.

  st_mysql_audit::class_mask doesn't always work as it's supposed to.



Now my proposals for our audit API.

The 'radical' proposal :

make the notify() function return 'int'

freeze the AUDIT_INTERFACE_VERSION forever.

notify() expects the 'event_class' as a parameter. It does something 
with the classes it is aware of.

if it gets the class it doesn't know about - it should just return.

notify() at the moment is expected to get the server data from the 
'const void *event' sent to it as a paramter,

Which is in my opinion inconvenient for everybody.
My idea is to provide a set of functions like 
'ap_get_query_string(MYSQL_THD thd, const void *event)', or

'ap_get_connection_id(MYSQL_THD thd, const void *event)'.
That set can be extended in newer versions of the server, but old 
functions never change.
If a plugin connects to the old server where that new possibilities are 
not implemented for some reason,

these function still available, just do nothing.
Plugin can check if some functions are available with the server it 
connected to.



We provide the plugin developer with an 'audit_plugin_adaptor.c' file, 
that is built
inside the plugin. It checks the version of the server that loads the 
plugin and links or
implements the 'ap_*' functions respectively. Also it provides proper 
structures for the
connecting server and interceipts notification's call to prepare it for 
the developer's notify() function.



That way the plugin once built can work with variety of servers. And 
getting newer 'audit_plugin_adaptor.c'
is going to make your plugin working with the newer versions of a server 
(be it MariaDB or MySQL).



Less radical proposition is to mimic MySQL's API, to make the plugin 
compilable both in MySQL and MariaDB
environment. So we get rid of the 'maria_declare_plugin', leaving only 
'mysql_declare_plugin'.

The present idea of having both doesn't work well.


Last option i see - get rid of the 'mysql_declare_plugin', leaving only 
'maria_declare_plugin' and
forget about the idea that same plugin is used in MySQL and MariaDB at 
the same time.
In this case i'd provide the set of the beforementioned 'ap_*' functions 
for the plugin to
get server data. As before i belive it'd be convenient, we'd never need 
to check the plugin
API version, and once built the plugin can work with many versions of 
MariaDB. And

we don't need the 'adaptor.c' file as the proper .h file seem to be enough.


Best regards.
HF

___
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