List:Commits« Previous MessageNext Message »
From:Luís Soares Date:March 4 2010 11:56pm
Subject:Re: bzr commit into mysql-next-mr-bugfixing branch (luis.soares:2976)
Bug#51426
View as plain text  
Hi Andrei,

On Thu, 2010-03-04 at 21:22 +0200, Andrei Elkin wrote:
> Luis, hello.
> 
> A nice analysis!

Thank you!

> I wondered on #rep how @aux := 1 can be interpreted as assigning of a signed...
> Value transition from  10294947273192243200 to 1 should not change the sign flag.
> But anyway that's how it is.

OK.

> The code part is okay.

Good.

> Still few suggestions to the tests.

I am all hears!

> > --Boundary_(ID_VKekphjkTnTFTUZBC3o+pA)
> > MIME-version: 1.0
> > Content-type: text/plain; CHARSET=US-ASCII
> > Content-transfer-encoding: 7BIT
> > Content-disposition: inline
> >
> > #At
> file:///home/lsoares/Workspace/bzr/work/bugfixing/51426/mysql-next-mr-bugfixing/ based on
> revid:alik@stripped
> >
> >  2976 Luis Soares	2010-02-25
> >       BUG#51426: overflow for auto_increment column causes slave to stop

[snip]

> > +
> > +#####################################################################
> > +#
> > +# BUG#51426: overflow for auto_increment column causes slave to stop
> 
> As auto_inc is irrelevant

Yes. Do you want me to change the synopsis?

> 
> > +#
> > +#####################################################################
> > +-- source include/master-slave-reset.inc
> > +-- connection master
> > +
> > +CREATE TABLE t1 ( c1 INT AUTO_INCREMENT, c2 BIGINT, PRIMARY KEY (c1));
> 
> auto_increment does not have to be specified (won't lead to false
> speculations, ... as well as few bytes less, which is good).
> One attribute is just enough.
> And
> 
>   CREATE TABLE t1 ( c INT, PRIMARY KEY (c)) engine=myisam.
> 
> yes, let's prepare for times of the default myisam might be over soon with
> `engine=myisam'. 
> (OTH if it changed the test wouldn't fail because the query wouldn't be
> replicated and the diff_table would be okay; but we would miss the test
> then entirely).

Ok, yes! I agree.

> > +
> > +# offending trigger that would reset the unsigned flag for aux before
> > +# binlogging of User_var_log_event would take place.
> > +CREATE TRIGGER tr1 AFTER INSERT ON t1 FOR EACH ROW SET @aux = 1 ;
> 
> Please change to @aux = -1 which would "rationalize" singness occurence.
> The fact of @aux dropped the unsigned flag when the value transits 
> from 
> 
> > +SET @aux = 10294947273192243200;
> 
> to 1 is somehow puzzling.

Ok. Done!

> > +-- error ER_DUP_ENTRY
> > +INSERT INTO t1 VALUES (@aux, 1) , (20030914140121, 2);
> 
> Oh, yes could we change it to
> 
> SET @aux = 10294947273192243200;
> SET @aux1 = @aux;
> INSERT INTO t1 VALUES (@aux), (@aux1);
> 
> to make -- error ER_DUP_ENTRY rather obvious.

Then why not: INSERT INTO t1 VALUES (@aux), (@aux); ?
I think this is even more obvious!

I can only say that in this commit, I kept the test case as 
close as the one in the bug report. But I agree, that we can make
it more intuitive.

A(n) (incremental) commit addressing your requests may be 
found at:
 - http://lists.mysql.com/commits/102382

Thanks for reviewing it.

Regards,
Luís


Thread
bzr commit into mysql-next-mr-bugfixing branch (luis.soares:2976)Bug#51426Luis Soares25 Feb
Re: bzr commit into mysql-next-mr-bugfixing branch (luis.soares:2976)Bug#51426Luís Soares5 Mar
Re: bzr commit into mysql-next-mr-bugfixing branch (luis.soares:2976)Bug#51426Libing Song7 Mar
  • Re: bzr commit into mysql-next-mr-bugfixing branch (luis.soares:2976)Bug#51426Luís Soares8 Mar