This piece of the code in Item_func_or_sum::agg_item_set_converter:
if (!conv && ((*arg)->collation.repertoire == MY_REPERTOIRE_ASCII))
conv= new (thd->mem_root) Item_func_conv_charset(thd, *arg, coll.collation, 1);
was wrong because:
1. It could change Item_cache to Item_func_conv_charset
(with the old Item_cache in args[0]).
Such Item type change is not always supported, e.g.
the code in Item_singlerow_subselect::reset() expects only Item_cache,
to be able to call Item_cache::set_null(). So it erroneously
reinterpreted Item_func_conv_charset to Item_cache and called
a non-existing method set_null(), which crashed the server.
2. The 1 in the last parameter to Item_func_conv_charset() was also
a problem. In MariaDB versions where the reported query did not crash,
it erroneously returned "empty set" instead of one row, because
the 1 made subselects execute too earlier and return NULL.
Fix:
1. Removing the above two lines from Item_func_or_sum::agg_item_set_converter()
2. Adding the repertoire test inside the constructor of Item_func_conv_charset,
so it now detects itself as "safe" in more cases than before.
This is needed to avoid new "Illegal mix of collations" after
removing the wrong code in various scenarios when character set
conversion from pure ASCII happens, including the reported scenario.
So now this sequence:
Item_cache -> Item_func_concat
is replaced to this compatible sequence (the top Item is still Item_cache):
new Item_cache -> Item_func_conv_charset -> Item_func_concat
Before the fix it was replaced to this incompatible sequence:
Item_func_conv_charset -> old Item_cache -> Item_func_concat
This change takes into account a column's GENERATED ALWAYS AS
expression dependcy on sql_mode's PAD_CHAR_TO_FULL_LENGTH and
NO_UNSIGNED_SUBTRACTION flags.
Indexed virtual columns as well as persistent generated columns are
now not allowed to have such dependencies to avoid inconsistent data
or index files on sql_mode changes.
So an error is now returned in cases like this:
CREATE OR REPLACE TABLE t1
(
a CHAR(5),
v VARCHAR(5) AS (a) PERSISTENT -- CHAR->VARCHAR or CHAR->TEXT = ERROR
);
Functions RPAD() and RTRIM() can now remove dependency on
PAD_CHAR_TO_FULL_LENGTH. So this can be used instead:
CREATE OR REPLACE TABLE t1
(
a CHAR(5),
v VARCHAR(5) AS (RTRIM(a)) PERSISTENT
);
Note, unlike CHAR->VARCHAR and CHAR->TEXT this still works,
not RPAD(a) is needed:
CREATE OR REPLACE TABLE t1
(
a CHAR(5),
v CHAR(5) AS (a) PERSISTENT -- CHAR->CHAR is OK
);
More sql_mode flags may affect values of generated columns.
They will be addressed separately.
See comments in sql_mode.h for implementation details.
It's better to prohibit pushdown of conditions that involve
regexp_substr() and regexp_replace() into materialized derived
tables / views until proper implementations of the get_copy()
virtual method are provided for those functions.
1. use Regexp_processor_pcre::set_recursion_limit() to set the
recursion limit depending on the current available stack size
2. make pcre stack frame to be estimated no less than 500 bytes.
sometimes pcre estimates it too low, even though the manual
says 500+16 bytes (it was estimated only 188 for me, actual
frame size was 512).
3. do it for embedded too
The bug happens because of a combination of unfortunate circumstances:
1. Arguments args[0] and args[2] of Item_func_concat point recursively
(through Item_direct_view_ref's) to the same Item_func_conv_charset.
Both args[0]->args[0]->ref[0] and args[2]->args[0]->ref[0] refer to
this Item_func_conv_charset.
2. When Item_func_concat::args[0]->val_str() is called,
Item_func_conv_charset::val_str() writes its result to
Item_func_conc_charset::tmp_value.
3. Then, for optimization purposes (to avoid copying),
Item_func_substr::val_str() initializes Item_func_substr::tmp_value
to point to the buffer fragment owned by Item_func_conv_charset::tmp_value
Item_func_substr::tmp_value is returned as a result of
Item_func_concat::args[0]->val_str().
4. Due to optimization to avoid memory reallocs,
Item_func_concat::val_str() remembers the result of args[0]->val_str()
in "res" and further uses "res" to collect the return value.
5. When Item_func_concat::args[2]->val_str() is called,
Item_func_conv_charset::tmp_value gets overwritten (see #1),
which effectively overwrites args[0]'s Item_func_substr::tmp_value (see #3),
which effectively overwrites "res" (see #4).
This patch does the following:
a. Changes Item_func_conv_charset::val_str(String *str) to use
tmp_value and str the other way around. After this change tmp_value
is used to store a temporary result, while str is used to return the value.
The fixes the second problem (without SUBSTR):
SELECT CONCAT(t2,'-',t2) c2
FROM (SELECT CONVERT(t USING latin1) t2 FROM t1) sub;
As Item_func_concat::val_str() supplies two different buffers when calling
args[0]->val_str() and args[2]->val_str(), in the new reduction the result
created during args[0]->val_str() does not get overwritten by
args[2]->val_str().
b. Fixing the same problem in val_str() for similar classes
Item_func_to_base64
Item_func_from_base64
Item_func_weight_string
Item_func_hex
Item_func_unhex
Item_func_quote
Item_func_compress
Item_func_uncompress
Item_func_des_encrypt
Item_func_des_decrypt
Item_func_conv_charset
Item_func_reverse
Item_func_soundex
Item_func_aes_encrypt
Item_func_aes_decrypt
Item_func_buffer
c. Fixing Item_func::val_str_from_val_str_ascii() the same way.
Now Item_str_ascii_func::ascii_buff is used for temporary value,
while the parameter passed to val_str() is used to return the result.
This fixes the same problem when conversion (from ASCII to e.g. UCS2)
takes place. See the ctype_ucs.test for example queries that returned
wrong results before the fix.
d. Some Item_func descendand classes had temporary String buffers
(tmp_value and tmp_str), but did not really use them.
Removing these temporary buffers from:
Item_func_decode_histogram
Item_func_format
Item_func_binlog_gtid_pos
Item_func_spatial_collection:
e. Removing Item_func_buffer::tmp_value, because it's not used any more.
f. Renaming Item_func_[un]compress::buffer to "tmp_value",
for consistency with other classes.
Note, this patch does not fix the following classes
(although they have a similar problem):
Item_str_conv
Item_func_make_set
Item_char_typecast
They have a complex implementations and simple swapping between "tmp_value"
and "str" won't work. These classes will be fixed separately.
The problem lies in how CURRENT_ROLE is defined. The
Item_func_current_role inherits from Item_func_sysconst, which defines
a safe_charset_converter to be a const_charset_converter.
During view creation, if there is no role previously set, the current_role()
function returns NULL.
This is captured on item instantiation and the
const_charset_converter call subsequently returns an Item_null.
In turn, the function is replaced with Item_null and the view is
then created with an Item_null instead of Item_func_current_role.
Without this patch, the first SHOW CREATE VIEW from the testcase would
have a where clause of WHERE role_name = NULL, while the second SHOW
CREATE VIEW would show a correctly created view.
The same applies for the DATABASE function, as it can change as well.
There is an additional problem with CURRENT_ROLE() when used in a
prepared statement. During prepared statement creation we used to set
the string_value of the function to the current role as well as the
null_value flag. During execution, if CURRENT_ROLE was not null, the
null_value flag was never set to not-null during fix_fields.
Item_func_current_user however can never be NULL so it did not show this
problem in a view before. At the same time, the CURRENT_USER() can not
be changed between prepared statement execution and creation so the
implementation where the value is stored during fix_fields is
sufficient.
Note also that DATABASE() function behaves differently during prepared
statements. See bug 25843 for details or commit
7e0ad09edf
This change is a backport from 10.0 to 5.5 for:
1. The full patch for:
MDEV-4841 Wrong character set of ADDTIME() and DATE_ADD()
9adb6e991e
2. A small fragment of:
MDEV-5298 Illegal mix of collations on timestamp
03f6778d61
which overrides Item_temporal_hybrid_func::cmp_type(),
and adds a new line into cache_temporal_4265.result.
This reverts commit 035a5ac62a.
Two minor problems and one regression:
1. caching the value in str_result. Other Item methods may use it,
destroying the cache. See, for example, Item::save_in_field, where
str_result is moved to use a local buffer (this failed main.grant)
2. Item_func_conv_charset::safe is now set too late, it's initialized
only in val_str() but checked before that, this failed many tests
in optimized builds.
to fix 1 - use tmp_result instead of str_result, to fix 2, use
the else branch in the Item_func_conv_charset constructor to set
safe purely from charset properties.
But this introduces a regression, constant strings can no longer be
converted, say, from utf8 to latin1 (because 'safe' will be false).
This fails few tests too. There is no way to fix it without reverting
the commit and converting constants, as before, in the constructor.
MDEV-10850 Wrong result for WHERE .. (f2=TO_BASE64('test') OR f2=TO_BASE64('TEST'))
Problem N1: MDEV-10425
Item_func_{md5|sha|sha2}::fix_length_and_dec() changed args[0]->collation
to force binary comparison in args[0]->eq().
It was done to treat e.g. MD5('a') and MD5('A') as different values.
It is wrong for a Item_func_xxx to modify its arguments.
Item_func_conv_charset did not expect that and crashed on assert.
Problem N2: MDEV-10850
Item_func_to_base64, Item_func_password, Item_func_hex are also case sensitive
hash functions, but they did not compare their arguments as binary.
Solution:
- Removing the code changing args[0]->collation
- Introducing Item_str_ascii_checksum_func as a common parent
for Item_func_{md5|sha|sha2|password|hex|to_base64}
and overriding its eq() method to compare arguments binary.
* revert part of the db7edfe that moved calculations from
fix_fields to val_str for Item_func_sysconst and descendants
* mark session state dependent functions in check_vcol_func_processor()
* re-run fix_fields for all such functions for every statement
* fix CURRENT_USER/CURRENT_ROLE not to use Name_resolution_context
(that is allocated on the stack in unpack_vcol_info_from_frm())
Note that NOW(), CURDATE(), etc use lazy initialization and do *not*
force fix_fields to be re-run. The rule is:
* lazy initialization is *not* allowed, if it changes metadata (so,
e.g. DAYNAME() cannot use it)
* lazy initialization is *preferrable* if it has side effects (e.g.
NOW() sets thd->time_zone_used=1, so it's better to do it when
the value of NOW is actually needed, not when NOW is simply prepared)
- Force usage of () around complex DEFAULT expressions
- Give error if DEFAULT expression contains invalid characters
- Don't use const_charset_conversion for stored Item_func_sysconf expressions
as the result is not constaint over different executions
- Fixed Item_func_user() to not store calculated value in str_value
