Hi, Aleksey!
Summary: looks ok, but please move clear_error() to the caller
(ha_innodb.cc) and remove the assert.
On Jul 04, Aleksey Midenkov wrote:
> revision-id: 4670c85a48c (mariadb-10.4.5-47-g4670c85a48c)
> parent(s): 0f55a9eb73b
> author: Aleksey Midenkov <mide...@gmail.com>
> committer: Aleksey Midenkov <mide...@gmail.com>
> timestamp: 2019-06-27 18:05:25 +0300
> message:
>
> MDEV-16222 Assertion `0' failed in row_purge_remove_sec_if_poss_leaf on table
> with virtual columns and indexes
>
> Cause
> Stale thd->m_stmt_da->m_sql_errno which is from different invocation.
>
> Fix
> Reset error state before attempt to open table.
>
> diff --git a/mysql-test/suite/innodb/r/purge_secondary_mdev-16222.result
> b/mysql-test/suite/innodb/r/purge_secondary_mdev-16222.result
> new file mode 100644
> index 00000000000..30e8f9800fb
> --- /dev/null
> +++ b/mysql-test/suite/innodb/r/purge_secondary_mdev-16222.result
> @@ -0,0 +1,34 @@
...
> +select * from t1 into outfile 'load.data';
> +Warnings:
> +Warning 1287 '<select expression> INTO <destination>;' is deprecated
> and will be removed in a future release. Please use 'SELECT <select list>
> INTO <destination> FROM...' instead
See the warning, better not to use the deprecated syntax, unless you
specifically want to test it,
> +load data infile 'load.data' replace into table t1;
> +set debug_sync= "now WAIT_FOR latch_released";
> +set global debug_dbug= "-d,ib_purge_virtual_mdev_16222_1";
> diff --git a/sql/sql_class.cc b/sql/sql_class.cc
> index d65b6b8ed8d..cb73b7db2de 100644
> --- a/sql/sql_class.cc
> +++ b/sql/sql_class.cc
> @@ -4762,7 +4762,10 @@ TABLE *open_purge_table(THD *thd, const char *db,
> size_t dblen,
> DBUG_ASSERT(!error || !ot_ctx.can_recover_from_failed_open());
>
> if (unlikely(error))
> + {
> close_thread_tables(thd);
> + thd->clear_error();
We use thd->clear_error() in many places, but over years there were
quite a few problems with it, and now I'd prefer to avoid it whenever
possible.
The thing is, you want to remove a specific error on a specific code
path, but thd->clear_error() removes all errors, and no matter how you
got here.
Generally intercepting errors with Internal_error_handler is a much
safer approach.
In this case, though, thd->clear_error() might be okay, but better move
it up the stack, do it in the caller when there can be only one way to
reach this code. Not in a kind-of-generically-looking open table helper.
> + }
>
> DBUG_RETURN(error ? NULL : tl->table);
> }
> diff --git a/sql/table.cc b/sql/table.cc
> index 9ac167b6adb..93c2c5e88a8 100644
> --- a/sql/table.cc
> +++ b/sql/table.cc
> @@ -8240,6 +8240,7 @@ int TABLE::update_virtual_fields(handler *h,
> enum_vcol_update_mode update_mode)
>
> int TABLE::update_virtual_field(Field *vf)
> {
> + DBUG_ASSERT(!in_use->is_error());
Yes, that's what I mean above. There were cases when
update_virtual_field() was called when is_error() was true and
is_error() introduced a bug.
I don't quite remember details, but it was something like, after the
error has happened somewhere, on the way to returning an error, it was
calling update_virtual_field() and some code in a function down the
stack was, like
some_function_which_returns_void(....);
if (thd->is_error()) // the error inside a function
{ ... handle it ... }
and the error in this case was not caused by anything in the function,
but existed from before.
The point is, is_error() and clear_error() are global, and you're
generally interested in local state. Whether _this block_ caused an
error. Clear the error caused by _this function call_. And so on, not
clear an error that might've happened some time somewhere at the
undefined point in the past before this code line was reached.
> Query_arena backup_arena;
> DBUG_ENTER("TABLE::update_virtual_field");
> in_use->set_n_backup_active_arena(expr_arena, &backup_arena);
Regards,
Sergei
VP of MariaDB Server Engineering
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
