List:Commits« Previous MessageNext Message »
From:Davi Arnaut Date:January 30 2008 1:37pm
Subject:Re: bk commit into 5.1 tree (davi:1.2661) BUG#32633
View as plain text  
Alexander Nozdrin wrote:
> Hi Davi,
> 
> this is just a summary of my thoughts about the bug and the patch.
> 
> The bug has the following parts:
> 
>   1. sql_mode enum was extended, but mysql.proc, mysql.event
>      were not updated. Strictly speaking, in order to fix the bug,
>      you only have to fix mysql_system_tables.sql script.

The patch updates the tables by changing the table definition in the
mysql_system_tables.sql script.

>   2. There is no test case checking that when a new sql_mode value
>      is added, it's added/registered in all needed places.

How to test that? I've can't come up with a test that will fail when a
new sql_mode is added.

>      We have two options here:
> 
>        - refactor the system database so that it does not care about
>          sql_mode values -- that means, use strings for storing sql_mode,
>          instead of set. This seems to be quite a significant change for
>          5.1 release. However, may be it deserves a Worklog item for
>          6.0/6.1. Backward compatibility should be the main concern here.

Changing to string would be a incompatible change at this time.

> 
>        - create a test case(s), that would automatically check that it's
>          possible to create a routine/event with any sql_mode. If it is
>          doable, I think it should be made in scope of this bug report.

As I said, how to make the test fail?

> 
>   3. There is no check that event being created was actually successfully
>      written to the system database. I think, this deficiency should also
>      be addressed in this patch.

Huh? care to elaborate?

> 
>      I.e. if any Field::store() operation failed, CREATE EVENT
>      should throw an error (like stored routines do).
> 
> As I understand it, by changing store(int) to store(string) you tried to make
> the code more independent of particular values of sql_mode. This however is
> not the problem we try to solve -- we will never change existing sql_mode
> values. That would be absolutely backward incompatible change and the users
> would kill us. What we try to smooth is future extension of sql_mode set.

As I said, the store part was a minor cleanup..

Regards,

-- 
Davi Arnaut, Software Engineer
MySQL Inc, www.mysql.com

Are you MySQL certified?  www.mysql.com/certification
Thread
bk commit into 5.1 tree (davi:1.2661) BUG#32633Davi Arnaut28 Jan
  • Re: bk commit into 5.1 tree (davi:1.2661) BUG#32633Alexander Nozdrin29 Jan
    • Re: bk commit into 5.1 tree (davi:1.2661) BUG#32633Davi Arnaut29 Jan
  • Re: bk commit into 5.1 tree (davi:1.2661) BUG#32633Alexander Nozdrin30 Jan
    • Re: bk commit into 5.1 tree (davi:1.2661) BUG#32633Davi Arnaut30 Jan
      • Re: bk commit into 5.1 tree (davi:1.2661) BUG#32633Paul DuBois30 Jan
        • Re: bk commit into 5.1 tree (davi:1.2661) BUG#32633Davi Arnaut30 Jan
          • Re: bk commit into 5.1 tree (davi:1.2661) BUG#32633Paul DuBois30 Jan