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