Re: [Maria-developers] bc5c062b1d1: Don't try to open temporary tables if there are no temporary tables

Hi, Michael! On Apr 13, Michael Widenius wrote:
Why?
please don't write conditions like that. keep it as before, two statements: if (thd->has_temporary_tables()) error= thd->open_temporary_table(tables); if (!error && !tables->table)
please, never do asserts with &&-ed conditions, this should be DBUG_ASSERT(!tl->derived); DBUG_ASSERT(!tl->schema_table); DBUG_ASSERT(has_temporary_tables());
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org

Hi! On Thu, Apr 16, 2020 at 1:17 PM Sergei Golubchik <serg@mariadb.org> wrote:
Why?
Because it's quite slow to loop over all tables to check if could be a temporary tables in the default case where we don't have any temporary tables at all. This will improve performance for almost any table open. I have added this to the commit message
Why? It's perfectly clear and not worse than if (!thd->has_temporary_tables()) if (!(error= thd->open_temporary_table(tables)) && !tables->table))
Sorry, the above is worse and more error prone as you error may no be set when you test it.
ok, I can change that. Regards, Monty

Hi, Michael! On Apr 16, Michael Widenius wrote:
Ok, I see. You mean THD::open_temporary_tables(). Because THD::open_temporary_table() already had a check and was not doing any loops. And you moved the check out, making it the responsibility of a caller. This increases copy-pasting and is very error-prone because now everyone has to remember to call thd->has_temporary_tables() before calling thd->open_temporary_table(). A better solution would be to have a private method THD::open_temporary_table_int() that does what THD::open_temporary_table() does in your patch, and an inline method inline bool THD::open_temporary_table(TABLE_LIST *tl) { return has_temporary_tables() && open_temporary_table_int(tl); } This way callers wouldn't need to be changed at all, while THD::open_temporary_tables() could use open_temporary_table_int() directly.
of course it's not worse. It's exactly identical. I said it's worse than what's below.
See open_and_process_table(), it is. Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
participants (2)
-
Michael Widenius
-
Sergei Golubchik