Hi, Nirbhay!
> > > diff --git a/mysql-test/r/temp_table.result
> > b/mysql-test/r/temp_table.result
> > > index ee0b3ab..94b02a6 100644
> > > --- a/mysql-test/r/temp_table.result
> > > +++ b/mysql-test/r/temp_table.result
> > > @@ -308,3 +308,185 @@ show status like 'com_drop%table';
> > > +DROP TABLE temp_t1;
> > > +# Cleanup
> > > +DROP DATABASE temp_db;
> > > +# MDEV-5535: End of tests
> >
> > What's that? You have lots of tests for temporary tables - not that I mind
> > the number, but they are mostly unrelated to MDEV-5535. Only the one last
> > test is about the new feature. I'd expect it to be the other way around -
> > many tests for the new functionality and few, if at all - for the
> > unmodified
> > behavior.
> >
> These generic tests were added to cover more possible use cases as the
> patch modified a good fraction of existing code. I have now moved
> these unrelated tests to a separate commit and added some more tests
> specific to MDEV-5535 in reopen_temp_table.test.
Good idea. I hope your separate commit with unrelated tests is *before*
the MDEV-5535 commit, so that one could checkout before MDEV-5535, see
unrelated tests pass, then checkout MDEV-5535 and see how unrelated
tests still pass.
> > > diff --git a/mysql-test/suite/handler/handler.inc
> > b/mysql-test/suite/handler/handler.inc
> > > index c71dc53..ca67b62 100644
> > > --- a/mysql-test/suite/handler/handler.inc
> > > +++ b/mysql-test/suite/handler/handler.inc
> > > @@ -489,7 +489,7 @@ handler t1 open as a1;
> > > handler a1 read a=(1);
> > > handler a1 read a next;
> > > handler a1 read a next;
> > > ---error ER_CANT_REOPEN_TABLE
> > > +#--error ER_CANT_REOPEN_TABLE
> >
> > remove it, don't comment. And, please, fix the comment before this test.
>
> Removed. What comment are you referring to?
This test in the handler.inc that you've edited. It has a comment before
the test starts:
#
# Bug#30882 Dropping a temporary table inside a stored function may
# cause a server crash
#
# Test HANDLER statements in conjunction with temporary tables. While the temporary table
# is open by a HANDLER, no other statement can access it.
#
You've fixed temporary tables, so this comment is no longer true.
> > > select a,b from t1;
> > > handler a1 read a prev;
> > > handler a1 read a prev;
> > > diff --git a/sql/temporary_tables.h b/sql/temporary_tables.h
> > > new file mode 100644
> > > index 0000000..c345bea
> > > --- /dev/null
> > > +++ b/sql/temporary_tables.h
...
> > > +class Temporary_tables {
...
> > > +private:
> > > + THD *m_thd;
> >
> > Hmm, is that necessary? Temporary_tables class is instantiated in only one
> > place - inside the THD. So Temporary_tables always belongs to its thd, and
> > storing a point to the parent THD in the Temporary_tables you basically add
> > another void* to already big THD class, while thid pointer stores no useful
> > information at all.
> >
> > A hackish way to fix is is to replace m_thd with
> >
> > (((char*)this) - offsetof(THD, temporary_tables))
> >
> > I'm not suggesting this hack, it is just to prove that m_thd is redundant.
> > I hope you'll find a nicer and safer solution :)
>
> How about current_thd in place of m_thd?
Better not. Svoj and Monty has recently done a lot of work removing
current_thd from many places, no need to introduce new ones.
Besides, this simply reading THD pointer from somewhere, while, in fact,
it's unambigously defined by 'this'.
Can you use TABLE::in_use pointer?
Or, may be, inherit THD from
Temporary_tables, and then thd==this :)
Or that pointer arithmetics, even (as above).
Regards,
Sergei
Chief Architect MariaDB
and security@mariadb.org