Hi Serg!
On Fri, Jun 3, 2016 at 7:20 AM, Sergei Golubchik
Hi, Nirbhay!
On May 26, Nirbhay Choubey wrote:
+/* + Create a temporary table, open it and return the TABLE handle. + + @param hton [IN] Handlerton + @param frm [IN] Binary frm image + @param path [IN] File path (without extension) + @param db [IN] Schema name + @param table_name [IN] Table name + @param open_in_engine [IN] Whether open table in SE + @param created [OUT] Whether table was created?
can it ever happen for *create==true but a return value is NULL? or the other way around? how?
Yes, but not the other way around. For instance, if the table was created successfully, but the subsequent malloc failed while an attempt was made to allocate TABLE object later in open_temporary_table().
But do you need to handle this practically impossible case specially? I'd say - if that happens, just drop the table (or even don't drop it, it'll be removed on disconnect anyway) and pretend the whole never happened (return NULL). I mean, you don't need a separate *created pointer.
I have update the function to remove the share from the list and free it in case it fails to open a table instance.
+ if (wait_for_prior_commit())
you're doing it in almost every method. And when one method calls another, you're doing it twice. Or thrice. Isn't is too much? (I don't know)
I added these additional waits to fix some failing replication tests. But, whit new TMP_TABLE_SHARE approach, this could have changed. I have now reverted them to the original state and will run rpl test to assure if we do not need additional waits.
And?
I found some failing rpl tests. Fixed.
+ free_io_cache(table);
don't forget to remove free_io_cache() calls when rebasing your work on top of the latest 10.2 (they were removed from close_temporary() in 260dd476b05 commit)
Okay
Unless you've rebased already, it makes sense to do it now - as you're apparently almost ready to push
Done. Best, Nirbhay
Regards, Sergei Chief Architect MariaDB and security@mariadb.org