Hi Sergei,
On 01/23/2017 07:50 PM, Sergei Golubchik wrote:
> Hi, Alexander!
>
> On Jan 11, Alexander Barkov wrote:
>> commit adf6bb91e349e8983b146cbfcaaa002edff54fe5
>> Author: Alexander Barkov <b...@mariadb.org>
>> Date: Wed Jan 11 13:42:52 2017 +0400
>>
>> MDEV-11134 Assertion `fixed' failed in
>> Item::const_charset_converter(THD*, CHARSET_INFO*, bool, const char*)
>>
>> Problem: Item_param::basic_const_item() returned true when fixed==false.
>> This unexpected combination made Item::const_charset_converter() crash
>> on asserts.
>>
>> Fix:
>> - Changing all Item_param::set_xxx() to set "fixed" to true.
>> This fixes the problem.
>> - Additionally, changing all Item_param::set_xxx() to set
>> Item_param::item_type, to avoid duplicate code, and for consistency,
>> to make the code symmetric between different constant types.
>> Before this patch only set_null() set item_type.
>> - Moving Item_param::state and Item_param::item_type from public to
>> private,
>> to make sure easier that these members are in sync with "fixed" and to
>> each other.
>> - Adding a new argument "unsigned_arg" to Item::set_decimal(),
>> and reusing it in two places instead of duplicate code.
>> - Adding a new method Item_param::fix_temporal() and reusing it in two
>> places.
>> - Adding methods is_no_value(), is_long_data_value(), is_int_value(),
>> instead of direct access to Item_param::state.
>
> Looks ok to push.
>
> Stylistic comments:
> * too many fixed= true in all set_* methods. I'd rather added a method
> fix_type(), to be used like
>
> - type= Item::STRING_ITEM
> + fix_type(Item::STRING_ITEM)
>
> and in this method I'd { type=type_arg; fixed= true }.
>
> * names like is_no_value/is_int_value/etc look strange, I'd
> use has_no_value, has_int_value, etc
Thanks! Addressed your suggestions and pushed.
>
> Regards,
> Sergei
> Chief Architect MariaDB
> and secur...@mariadb.org
>
_______________________________________________
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help : https://help.launchpad.net/ListHelp
