Re: bk commit into 5.1 tree (davi:1.2661) BUG#32633

[Alexander Nozdrin]

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.

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

     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.

       - 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.

  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.

     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.

Thanks!

-- 
Alexander Nozdrin, Software Developer
MySQL AB, Moscow, Russia, www.mysql.com