Code review comment for lp:~sergei.glushchenko/percona-server/5.6-ps-bug1182535

Hi Sergei,

On Fri, 12 Jul 2013 14:03:35 -0000, Sergei Glushchenko wrote:> Hi Alexey,
>
>> 1. Now we do query rewriting even if we don't need it ('rewrite' will be true if all of opt_log, opt_log_raw and thd->slave_thread are false).
> Don't we need to rewrite queries for audit plugin? Is it secure to send raw passwords to it?

My point was that we only need to rewrite queries only if there's an audit plugin installed. With the current patch we will be rewriting queries even if no general log or audit plugins are enabled.

>
>> 2. If the general log is disabled, we will still do general logging, because the "if (!thd->slave_thread)" check will pass.
> No, we don't since opt_log is checked in general_log_write.
>

Right.

>> 3. If both the general log and --log-raw are enabled, we will be general-logging queries twice, once in dispatch_command() and once in mysql_parse()
> Yes, I think the best choice is to remove logging from dispatch_command().
>

Sounds good.

>> Do we really need to change anything in sql_parse.cc? I.e. what problem are you trying to solve?
> Yes, we do. This change simply allows us to still notify audit plugin even if opt_log in not specified. This is what entire bug is about.
>

OK, I see. But again, I don't like that we do unnecessary work even if no logging or audit plugins are enabled. That's a rather serious performance issue. What about introducing mysql_audit_general_log_enabled() and calling it to see if we do need to rewrite query for an audit plugin?