Hi, Alexey! On May 08, Alexey Botchkov wrote: > revision-id: eb3e9cdbe86b5a68c3ec8a9f26165259e4ca7eec > (mariadb-10.3.6-125-geb3e9cd) > parent(s): bd09c5ca866e273b6cebbd9a15c51c82bfa3ac9b > committer: Alexey Botchkov > timestamp: 2018-05-08 14:19:55 +0400 > message: > > MDEV-15813 ASAN use-after-poison in hp_hashnr upon HANDLER READ on a > versioned HEAP table. > > Returns an error if we try to use a key in a query it declared unable to > handle. > > diff --git a/sql/share/errmsg-utf8.txt b/sql/share/errmsg-utf8.txt > index c55ac4f..7446a60 100644 > --- a/sql/share/errmsg-utf8.txt > +++ b/sql/share/errmsg-utf8.txt > @@ -7915,3 +7915,5 @@ ER_UPDATED_COLUMN_ONLY_ONCE > eng "The column %`s.%`s cannot be changed more than once in a single > UPDATE statement" > ER_EMPTY_ROW_IN_TVC > eng "Row with no elements is not allowed in table value constructor > in this context" > +ER_KEY_DOESNT_SUPPORT > + eng "%s index %`s does not support this operation"
I think this can be safely pushed into 10.2. But not earlier, we can only add new error messages in the latest GA version, not in previos GA's. > diff --git a/sql/sql_handler.cc b/sql/sql_handler.cc > index 09883d8..12e4b5a 100644 > --- a/sql/sql_handler.cc > +++ b/sql/sql_handler.cc > @@ -661,12 +661,30 @@ mysql_ha_fix_cond_and_key(SQL_HANDLER *handler, > key_part_map keypart_map; > uint key_len; > > + if (ha_rkey_mode != HA_READ_KEY_EXACT && > + (table->file->index_flags(handler->keyno, 0, TRUE) & > + (HA_READ_NEXT | HA_READ_PREV | HA_READ_RANGE)) == 0) I suspect there can be hypothetical cases where an index supports some specific ha_rkey_mode values and has some specific flags, but not all of them, like you test here. But as the only real use case is HASH vs BTREE now, your check is fine. No need overengineer a complex test that we cannot possibly test now. > + { > + my_error(ER_KEY_DOESNT_SUPPORT, MYF(0), > + table->file->index_type(handler->keyno), keyinfo->name); > + return 1; > + } > + > if (key_expr->elements > keyinfo->user_defined_key_parts) > { > my_error(ER_TOO_MANY_KEY_PARTS, MYF(0), > keyinfo->user_defined_key_parts); > return 1; > } > + else if (key_expr->elements < keyinfo->user_defined_key_parts && > + (table->file->index_flags(handler->keyno, 0, TRUE) & > + HA_ONLY_WHOLE_INDEX)) > + { > + my_error(ER_KEY_DOESNT_SUPPORT, MYF(0), > + table->file->index_type(handler->keyno), keyinfo->name); > + return 1; Good point! > + } This is ok to push. Just two questions: 1. Why did you add it in sql_handler.cc and not in the class handler method, like ha_index_read_map or ha_index_next ? 2. Could you add tests for fulltext/rtree indexes too, please? 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