Hi, Sachin! On Mar 12, Sachin Setiya wrote:

revision-id: 1aa6d5bdbec (mariadb-10.4.3-63-g1aa6d5bdbec) parent(s): b5d2e8577ef author: Sachin Setiya committer: sachin timestamp: 2019-03-12 19:29:58 +0530 message:

MDEV-18888 Server crashes in Item_field::register_field_in_read_map upon... MODIFY COLUMN

Do Not create prefix field for long unique key

diff --git a/sql/table.cc b/sql/table.cc index 4da873b2e8f..5ca284fd8dd 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -3709,7 +3709,33 @@ enum open_frm_error open_table_from_share(THD *thd, TABLE_SHARE *share, Field *field= key_part->field= outparam->field[key_part->fieldnr - 1];

if (field->key_length() != key_part->length && - !(field->flags & BLOB_FLAG)) + !(field->flags & BLOB_FLAG) && + /* + MDEV-18888 + We will not make copy of the field because if we do this this this will + make this test case to fail + CREATE TABLE t1 ( + a CHAR(128), + b CHAR(128) AS (a), + c varchar(5000), + UNIQUE(c,b(64)) + ) ENGINE=InnoDB; + And the reason for this is because Item_func_hash will Item_field based on + this new field. But if this field is virtual field then since it is created + before calling parse_vcol_defs so it does not have field->vcol_info->expr item + initiated. And this will make segfault when field->vcol_info->expr is accessed. + So the question may be why similar unique with just virtual column is not crashing ? + CREATE TABLE t2 ( + a CHAR(128), + b CHAR(128) AS (a), + c char(128) as (a+b), + UNIQUE(a, c(64)) + ) ENGINE=InnoDB; + In this case also new prefix field will be created whose vcol_info->expr will be NULL. + But Since this key is not a long unique in innodb_base_col_setup field will be + table->field[i] which will have not null vcol_info->expr + */ + key_info->algorithm != HA_KEY_ALG_LONG_HASH)

don't put a comment (particularly a long one) inside the if(), move it to be before the if(), please. and I'd used a simpler comment, like "there's no need to create a prefix Field for HA_KEY_ALG_LONG_HASH indexes, as they implement prefixing via Iten_func_left anyway (see parse_vcol_defs()"

{ /* We are using only a prefix of the column as a key:

Regards, Sergei Chief Architect MariaDB and security@mariadb.org