Hi, Sergei!
On Fri, Dec 6, 2019 at 5:34 PM Sergei Golubchik <s...@mariadb.org> wrote:
> Hi, Aleksey!
>
> On Dec 05, Aleksey Midenkov wrote:
> > >
> > > > 3. Invalidate referenced shares on:
> > > >
> > > > - RENAME TABLE
> > > > - DROP TABLE
> > > > - RENAME COLUMN
> > > > - CREATE TABLE
> > > > - ADD FOREIGN KEY
> > > >
> > > > When foreign table is created or altered by the above operations
> > > > all referenced shares are closed. This blocks the operation while
> > > > any referenced shares are used (when at least one its TABLE
> > > > instance is locked).
> > >
> > > And this is the main question of this email:
> > > Why do you close referenced tables?
> >
> > Table cannot operate with wrong TABLE_SHARE::referenced_keys, so it
> > must be locked anyway whether referenced_keys are modified or
> > reloaded. The choice was made to reload as the first simplest
> > implementation. And unless we know real performance drawback I'd like
> > to keep it that way. Modifying TABLE_SHARE::referenced_keys requires
> > several different algorithms for the above operations.
>
> I feel that closing tables is a bit too heavy.
>
> By the way, do you open referenced tables for SELECT? Or does
> table->s->referenced_keys stay NULL?
>
Now it stays NULL, but next implementation with FRM files will see foreign
tables list referencing it and it will need to open their shares to update
its referenced_keys. I don't know if we need such level of independence as
to not open foreign tables when not needed. But yes, that could be a
feature to load foreign tables on demand.
>
> > > Minor comments below.
> > > > diff --git a/sql/handler.h b/sql/handler.h
> > > > index e913af1d15d..10984505f70 100644
> > > > --- a/sql/handler.h
> > > > +++ b/sql/handler.h
> > > > @@ -1030,6 +1031,15 @@ struct TABLE_SHARE;
> > > > struct HA_CREATE_INFO;
> > > > struct st_foreign_key_info;
> > > > typedef struct st_foreign_key_info FOREIGN_KEY_INFO;
> > > > +class Table_ident;
> > > > +class FK_list : public List<FOREIGN_KEY_INFO>
> > > > +{
> > > > +public:
> > > > + /* Get all referenced tables for foreign key fk_name. */
> > > > + bool get(THD *thd, std::set<Table_ident> &result, LEX_CSTRING
> &fk_name, bool foreign);
> > > > + /* Get all referenced or foreign tables. */
> > > > + bool get(THD *thd, std::set<Table_ident> &result, bool foreign);
> > >
> > > Seems unnecessary. Copying a list into a std::set _could_ be
> justified, if
> > > you'd later used it for quick checks "if this table belongs to a set" -
> > > something you cannot quickly do with a List.
> > >
> > > But as far as I can see you copy the List into std::set and then
> iterate
> > > this set. This doesn't make much sense, you can iterate the original
> > > list just fine.
> >
> > std::set does duplicate elimination. Iterating a list would require an
> > algorithm to skip the duplicates.
>
> Duplicates? Can there be duplicate foreign keys?
>
No, but duplicates of Table_ident can be, because multiple foreign keys can
reference same table.
> Regards,
> Sergei
> VP of MariaDB Server Engineering
> and secur...@mariadb.org
>
--
All the best,
Aleksey Midenkov
@midenok
_______________________________________________
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
