maria-developers team mailing list archive

[Nirbhay Choubey <nirbhay@xxxxxxxxxxx>]

Hi Serg!

On Fri, Jun 3, 2016 at 7:20 AM, Sergei Golubchik <serg@xxxxxxxxxxx> wrote:

> 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@xxxxxxxxxxx
>