Hi Sergei, It appeared to be very easy to fix all hybrid functions to use the same attribute aggregation code, and therefore get rid of duplicate implementations for Item_func_case_abbreviation2 and Item_func_case. This patch fixes: - The original problem reported in MDEV-9653 - An additional problem, see new comments about COALESCE and IF returning 1.1000000000000000000000000000000 instead of just 1.1 - A new problem that I found: MDEV-9752 Wrong data type for COALEASCE(?,1) in prepared statements Note, the patch does not cover LEAST/GREATEST. They have another version of their own aggregation code. I don't want to touch them in this fix. Let's do them separately. Note, I also removed Item_func_case::agg_str_lengths(). It was a dead code. Thanks. On 03/17/2016 03:06 PM, Alexander Barkov wrote:
Hi Sergei,
On 03/17/2016 01:14 PM, Sergei Golubchik wrote:
Hi, Alexander!
On Mar 17, Alexander Barkov wrote:
why is it NOT_FIXED_DEC? it seems to me that it has to be 0 here.
Perhaps my example in the comments made you think that it happens in a very special case with CAST AS UNSIGNED. It also happens in simpler cases:
SELECT CASE WHEN TRUE THEN COALESCE(NULL) ELSE 40 END;
I guess I should fix the example to this ^^^.
Yes, please.
More questions:
1. Perhaps my_decimal_length_to_precision() then? It's used not only in Item_func_case::agg_num_lengths. Quick grepping finds more potentially dangerous places. For example Item_decimal_typecast::print.
I could not find places where my_decimal_length_to_precision() can be called with (0,NOT_FIXED_DEC).
Item_decimal_typecast::print() is safe. Item_decimal_typecast::max_length and Item_decimal_typecast::decimals are set to valid values in constructor. The latter cannot be NOT_FIXED_DEC.
2. Why my_decimal_length_to_precision is only used in Item_func_case? How do other functions aggregate types? Perhaps CASE needs to be changed to work like other similar functions do?
There is some code duplication in hybrid type functions:
- Item_func_case::agg_num_lengths()
- Item_func::count_decimal_length(), which is called from Item_func_coalesce::fix_length_and_dec()
- Item_func_case_abbreviation2::fix_length_and_dec()
I earlier created a task for this for 10.2: https://jira.mariadb.org/browse/MDEV-9406
I found new bugs when replying to this letter:
SELECT COALESCE(COALESCE(NULL), 1.1); SELECT IF(0, COALESCE(NULL), 1.1);
return 1.1000000000000000000000000000000, which is wrong.
I'll come up with a new patch soon.
Thanks.
Regards, Sergei Chief Architect MariaDB and security@mariadb.org