Am 14.06.2018 um 14:19 schrieb Sergei Golubchik:
Hi, Oleksandr!
On Jun 14, Oleksandr Byelkin wrote:
Am 13.06.2018 um 12:08 schrieb Sergei Golubchik:
diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 9e7973b745c..f3cb85f01d3 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -4894,7 +4894,13 @@ int create_table_impl(THD *thd, file= mysql_create_frm_image(thd, orig_db, orig_table_name, create_info, alter_info, create_table_mode, key_info, key_count, frm); - if (!file) + /* + We have to check thd->is_error() here because it can be set by + Item::val* for example, and before it will be cought accidentally by + Item_func::fix_fields() of the next call. Now we removed the check + from Item_func::fix_fields() + */ and I still don't understand why do you need to check for thd->is_error() here because it catch some other errors which never will be checked and fixing this IMHO is other matter and in higher version. Some other errors like what?
Am 14.06.2018 um 11:44 schrieb Sergei Golubchik: like an error in val_* call due to some incompatibility of arguments during mysql_create_frm_image() as I remember. i.e. our tests will fail without this check because the error left unchecked till it is too late. I put TODO remove this tests, but without them it will not just work. Is there a test that fails without them? yes, completely unrelated tests, before the error will be caught in fix_fields() which was produced for unrelated parts of the query (and only if we have Item_func there). We just have no other mechanisms to catch error during val_* than check thd->is_error(). Wouldn't the original bug show up again? You're testing for
On Jun 14, Oleksandr Byelkin wrote: thd->is_error() and it might be unrelated coming from some earlier error, right?
I think no, because it is on the upper level. I.e. we tried to generate .frm and checking if it succeed before starting preparing other parts of the query, and actually try to execute full query if .frm was not created will lead eventually to crashes and other problems. in other words we stop execution when it is not too late because 1) it has no sens 2) it is impossible without success of the first part.
Regards, Sergei Chief Architect MariaDB and security@mariadb.org