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 security@mariadb.org