From: Sergey Vojtovich
Date: December 2 2010 8:06pm
Subject: Re: bzr commit into mysql-trunk branch (Li-Bing.Song:3350)
 Bug#48183
List-Archive: http://lists.mysql.com/commits/125859
Message-Id: <20101202200643.GA23770@june>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii

Hi Alfranio,

On Thu, Dec 02, 2010 at 06:33:04PM +0000, Alfranio Correia wrote:
> Hi Libing,
> 
> Sergey is right on his comments on the unsafe statements.
> 
> There are two flags. One that indicates that a statement is unsafe
> (i.e. unsafe_flags) and as such a warning message will be issued if
> the STMT format is in use and the execution will switch from
> statement to
> row if the MIXED format is in use.
> 
> The second flag (i.e binlog_unsafe_warning_flags) is a copy of the
> first and is used only when the STMT format is in use and triggers
> warning messages.
> 
> We set the "unsafe_flags" by calling
> 
> thd->lex->set_stmt_unsafe((LEX::BINLOG_STMT_UNSAFE_PLUGIN);
> 
> and in int THD::decide_logging_format(TABLE_LIST *tables)
> the unsafe_flags is automatically copied to binlog_unsafe_warning_flags
> if necessary.
> 
> There is no need to remove the comma in the enum as this is portable
> as far as I know but I don't have a strong opinion on that.
I have no strong opinion either. As well as I have no proof that trailing
comma is not portable.

But I think I saw patches removing trailing commas many times. Can't
remember exact patches, but likely against backup, Falcon, InnoDB or NDB.

Regards,
Sergey

> 
> Cheers.
> 
> On 12/02/2010 02:04 PM, Sergey Vojtovich wrote:
> >Hi Li-Bing,
> >
> >On Thu, Dec 02, 2010 at 12:44:00PM +0800, Libing Song wrote:
> >
> >...skip...
> >
> >>>>>Also, being non-replication guy, I'm a bit confused: you set unsafe
> >>>>>warning flag, but you don't really mark statement unsafe? If it is
> >>>>>done implicitely, could you point me to that code?
> >>>>binlog_query() calls issue_unsafe_warnings(). issue_unsafe_warnings()
> >>>>checks this flag and then generates different unsafe warnings.
> >>>Right, it generates different unsafe warnings. But I don't see how it
> >>>marks statement unsafe.
> >>First, lex->set_stmt_unsafe() are called by yacc.yy and some functions
> >>(eg. uuid, rand)when they are initializing.
> >Right. But we don't call set_stmt_unsafe() for MATCH()?
> >
> >>Second, after opening all tables related to a statement,
> >>decide_logging_format() are called to decide what binlog format should
> >>be used in current statement.
> >>decide_logging_format() will copy the unsafe flags marked by
> >>lex->set_stmt_unsafe to thd->binlog_unsafe_warning_flags if
> >>the statement has to be binlogged in statement format.
> >Please don't be blocked by this question. I'm just trying to educate
> >myself.
> >
> >Could you correct me where I'm wrong:
> >   I read bug report and I see that unsafe statements are to be logged
> >   in row format if binlog_format=mixed or cause a warning to be generated
> >   if binlog_format=statement.
> >   Also I see that it is suggested to mark MATCH() using plugin unsafe.
> >   I read your patch and see that a warning is generated if binlog_format
> >   is statement.
> >   I can see that you achieve it by setting binlog_unsafe_warning_flags.
> >   But in mixed mode you still log MATCH() in statement format, because
> >   binlog_stmt_flags is unaffected. It is contradicting to the requirement
> >   listed in the bug report.
> >
> >Probably it is a good idea to test mixed mode as well. At least to avoid
> >questions like this.
> >
> >Regards,
> >Sergey
> 
> 
> -- 
> MySQL Code Commits Mailing List
> For list archives: http://lists.mysql.com/commits
> To unsubscribe:    http://lists.mysql.com/commits?unsub=svoj@stripped
> 

-- 
Sergey Vojtovich <svoj@stripped>
MySQL AB, Software Engineer
Izhevsk, Russia, www.mysql.com