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.
preserve positions if the multi-update join is using tmp table:
* store positions in the tmp table if needed
JOIN::add_fields_for_current_rowid()
* take positions from the tmp table, not from file->position():
multi_update::prepare2()
Main changes:
- Changing the constructor to accept a CHARSET_INFO pointer, instead of an Item pointer
- Updating the bison grammar accordingly
Additional cleanups:
- Simplifying Item_func_set_collation::eq() by reusing Item_func::eq()
- Removing unused binary_keyword
TODO:
- Make get_thd_memroot() inline
- To do this, we need to reduce dependence of include files, especially
so that sql_class.h is not depending in item.h
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.