Hi, Sergei.

Could you, please remove that piece of fine creativity and thd->clear_error() too and use the error handler instead?

There's one puzzle. We need to fill the INFORMATION_SCHEMA.TABLES.TABLE_COMMENT field. If a table exists but corrupt, that field presently is filled with the error message. Like here (tx.MYD and tx.MYI files were removed, only tx.frm left): mysql> SELECT TABLE_SCHEMA, TABLE_NAME, TABLE_TYPE, ENGINE, ROW_FORMAT, TABLE_ROWS, DATA_LENGTH, TABLE_CO MMENT FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_NAME = 'tx'; +--------------+------------+------------+--------+------------+------------+-------------+-------------- --------------------+ | TABLE_SCHEMA | TABLE_NAME | TABLE_TYPE | ENGINE | ROW_FORMAT | TABLE_ROWS | DATA_LENGTH | TABLE_COMMENT | +--------------+------------+------------+--------+------------+------------+-------------+-------------- --------------------+ | test | tx | BASE TABLE | NULL | NULL | NULL | NULL | Can't find file: 'tx' (errno: 2) | +--------------+------------+------------+--------+------------+------------+-------------+----------------------------------+ 1 row in set, 5 warnings (0.00 sec) This field is set here: sql_show.cc:4899 (get_schema_tables_record()) if (res || info_error) { /* If an error was encountered, push a warning, set the TABLE COMMENT column with the error text, and clear the error so that the operation can continue. */ const char *error= thd->is_error() ? thd->stmt_da->message() : ""; table->field[20]->store(error, strlen(error), cs); if (thd->is_error()) { push_warning(thd, MYSQL_ERROR::WARN_LEVEL_WARN, thd->stmt_da->sql_errno(), thd->stmt_da->message()); thd->clear_error(); } } As we're supposed to block that error with the Error_handler, the error message gets lost there and cannot be recovered later in the get_schema_tables_record. So we either have stop showing the error message in that field, or we have to store it somewhere. The Error_handler looks like a good place to store that error message. But i can't figure out how to send it to the get_schema_tables_record() nicely. Another place is the TABLE_LIST * parameter. Seems to be convenient for eveything. Though the TABLE_LIST structure will get even more polluted. Sergei, what is your opinion here? Best regards. HF 17.02.2014 20:30, Sergei Golubchik wrote: Hi, Alexey! On Feb 14, Alexey Botchkov wrote:

...

...

But I don't understand that. Do you mean, old code did *not* suppress errors here? How could selects from I_S tables work without it? Here's how it works today:

get_all_tables() Trigger_error_handler err_handler; thd->push_internal_handler(&err_handler); fill_schema_table_from_frm get_table_share .... /* here we get the error of file not found */ my_message_sql THD::raise_condition /* here we test the err_handler */ /* but it doesn't react on that kind of errors */ mysql_audit_notify() stmt_da->set_error_status warning_info->push_warning / / .... /*error is returned back along the call stack */ / thd->clear_error(); /*which erases all the fileopen errors*/ /

So basically the error is launched and then erased. Which doesn't help with the plugin notifications. To fix that i added one more error handler. It's worse than that :) After thd->clear_error() the error flag is cleared, indeed. But all issued errors are still present in the warning list. I've looked into this about found this little gem: do_fill_table() function.

It actually copies Warning_info list and removes warnings from there!

Could you, please remove that piece of fine creativity and thd->clear_error() too and use the error handler instead?

Looking at the logic of do_fill_table(), all you need to do is to filter out all errors, but keep all warnings. Like

class Warnings_only_error_handler : public Internal_error_handler { public: bool handle_condition(THD *thd, uint sql_errno, const char* sqlstate, MYSQL_ERROR::enum_warning_level level, const char* msg, MYSQL_ERROR ** cond_hdl) { return level == MYSQL_ERROR::WARN_LEVEL_ERROR; } };

Regards, Sergei

Hi. As you didn't react on my last patch about it, i decided to remind: http://lists.askmonty.org/pipermail/commits/2014-February/005931.html Best regards. HF 19.02.2014 20:43, Sergei Golubchik wrote:

Hi, Alexey!

...

Hi, Sergei.

...

Could you, please remove that piece of fine creativity and thd->clear_error() too and use the error handler instead? There's one puzzle. This field is set here: sql_show.cc:4899 (get_schema_tables_record()) if (res || info_error) { /* If an error was encountered, push a warning, set the TABLE COMMENT column with the error text, and clear the error so that the operation can continue. */ const char *error= thd->is_error() ? thd->stmt_da->message() : ""; table->field[20]->store(error, strlen(error), cs);

if (thd->is_error()) { push_warning(thd, MYSQL_ERROR::WARN_LEVEL_WARN, thd->stmt_da->sql_errno(), thd->stmt_da->message()); thd->clear_error(); } }

As we're supposed to block that error with the Error_handler, the error message gets lost there and cannot be recovered later in the get_schema_tables_record. So we either have stop showing the error message in that field, or we have to store it somewhere.

The Error_handler looks like a good place to store that error message. But i can't figure out how to send it to the get_schema_tables_record() nicely. Another place is the TABLE_LIST * parameter. Seems to be convenient for everything. Though the TABLE_LIST structure will get even more polluted. Error handler looks perfect - it could have a char[] buffer and remember

On Feb 19, Alexey Botchkov wrote: there the first error message for SHOW TABLES.

But I agree that it's difficult to reach from get_schema_tables_record(). One option would be to use

THD::get_internal_handler()

it'll allow you to access the current error handler. The problem with this solution - there's no easy way to verify that you've got an object of the correct class. If someone has pushed another error handler after you, this will crash.

Another solution could to have a char* pointer in THD or TABLE_LIST and set it to point to this buffer. This will work even if there's another handler on top of yours. But it'll need a pointer in THD or TABLE_LIST.

I'd probably use the THD::get_internal_handler() approach and a test case for it (it'll make sure the calling convention is maintained). Seems silly to add another member to THD or TABLE_LIST that are used literally everywhere only to show an open table error in I_S.TABLES.

Regards, Sergei