← Back to team overview

maria-developers team mailing list archive

Re: c91ec05e01b: MDEV-20865 Store foreign key info in TABLE_SHARE

 

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?

> > 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?

Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx


Follow ups

References