maria-developers team mailing list archive

[jerome brauge <j.brauge@xxxxxxxxxxx>]

Thanks Simon, Sergei.

Initially , I've done this change only for sql_mode=oracle (where concat null and a string doesn’t return null) but as SQLServer has an option to have the same behavior (set CONCAT_NULL_YIELDS_NULL   ON|OFF), I was thinking to add a dedicated option.

How replication will work if we add/change SQL function (for MDEV-10142 PL/SQL Parser) if slave hasn't the same level ? I supposed that in this case, only row_based logging is safe.
Alexander, what is your point of view ?

Sergei, all works fine with a my_set_bits64().

Jérôme.


-----Message d'origine-----
De : Simon J Mudd [mailto:simon.mudd@xxxxxxxxxxx] 
Envoyé : mardi 27 décembre 2016 13:17
À : jerome brauge; serg@xxxxxxxxxxx
Cc : maria-developers@xxxxxxxxxxxxxxxxxxx
Objet : Re: [External] [Maria-developers] check_sql_mode


> On 27 Dec 2016, at 11:35, Sergei Golubchik <serg@xxxxxxxxxxx> wrote:
> 
> Hi, jerome!
> 
> On Dec 27, jerome brauge wrote:
>> Hello,
>> I'm trying to add an option to sql_mode.
>> For this, I'm adding in sql_class.h :
>> #define MODE_CONCAT_NULL_YIELDS_NULL    (1ULL << 32)
>> 
>> But now, my serveur doesn't start and show "Sysvar 'sql_mode' failed 'def_val <= my_set_bits(typelib.count)'"
>> The problem seems to be in sys_vars.ic (Sys_var_set) :
>> 
>>  {
>>    option.var_type|= GET_SET;
>>    global_var(ulonglong)= def_val;
>>    SYSVAR_ASSERT(typelib.count > 0);
>>    SYSVAR_ASSERT(typelib.count <= 64);
>>    SYSVAR_ASSERT(def_val <= my_set_bits(typelib.count));
>>    SYSVAR_ASSERT(size == sizeof(ulonglong));  }
>> 
>> Function my_set_bits works on 32bit and typelib.count may be up to 64.
>> In my case typelib.count is 33 and my_set_bits(33) returns 1, and the assert is false.
> 
> I'd say it's a bug.
> It seems that we didn't have any Sys_var_set variable with more than 
> 32 elements in the set yet, so it didn't show up.
> 
> To continue, I think, you can fix my_set_bits to work on ulonglongs.
> Or create a second my_set_bits64().
> 
> Btw, I suspect you'll see more sql_mode issues caused by you going 
> over 32 bits.

Slightly off topic but changes in sql_mode has the potential to cause problems with major version changes and incompatible behaviour.
So combining all sorts of new settings into this variable does not seem like a good idea to me. It’s already rather complex.

Note: don’t just think of how the server behaves with this change but also think of replication. Downstream slaves may not be running the same (major or minor) version of MariaDB (or MySQL) and sql_mode is fed into the binlog stream so will be interpreted by the SQL thread(s) of the downstream slaves too.

Consequently behavioural changes like this by adding new values may not be as simple as you expect.

I’d be much more comfortable using a new global variable which defines this behaviour, though that _may_ require the setting being propagated via the binlog stream if setting the value globally on the downstream slave is not good enough.

Just a thought.

Simon