← Back to team overview

maria-developers team mailing list archive

Re: 4b01d3aee60: MDEV-17082 Application-time periods: CREATE

 

Hi, Sergei!

On Fri, Feb 8, 2019 at 5:02 AM Sergei Golubchik <serg@xxxxxxxxxxx> wrote:

> Hi, Nikita!
>
> On Feb 08, Nikita Malyavin wrote:
> > On Fri, Feb 8, 2019 at 2:18 AM Sergei Golubchik <serg@xxxxxxxxxxx>
> wrote:
> > > I thought you've wanted it a CHECK constraint. You've quoted the
> > > standard proving your point of view.
> >
> > I've also quoted that it's name shouldn't match any existing
> > constraint name:)
>
> yes, we have this issue for all auto-generated names, and the usual
> workaround is to append _1, _2, etc until there's a free name.
>
> Okay, added to ALTER TABLE commit

> > > > > diff --git a/sql/table.h b/sql/table.h
> > > > > > --- a/sql/table.h
> > > > > > +++ b/sql/table.h
> > > > > > @@ -1730,6 +1747,9 @@ class IS_table_read_plan;
> > > > > >  /** The threshold size a blob field buffer before it is freed */
> > > > > >  #define MAX_TDC_BLOB_SIZE 65536
> > > > > >
> > > > > > +/** number of bytes read by uint2korr and sint2korr */
> > > > > > +#define korr2size 2
> > > > >
> > > > > No, that's silly. It's like #define TWO 2
> > > > > Of course korr2size is 2. And korr3size is 3, and korr4size is 4.
> > > >
> > > > Of course! That's the way to say "This variable was stored as a
> 2-byte
> > > > tuple".
> > >
> > > I mean, you didn't remove the magic number. It's still 2, and if you'd
> > > like to change it to be 3, for example, you'd still need to go and
> > > change the code everywhere.
> >
> > Oh, so You're really thinking on such refactoring, to change the sizes of
> > metainfo types  rapidly? Then that 👇makes sense:
>
> No, I don't. Changing storage format is very heavy and thus very rare
> operation.
>
> I was just using it as an example, to show that embedding a magical
> number in the name of a constant doesn't make it any less magical or the
> code any easier to understand. "Easy to change" is a good rule, even if
> you don't actually plan to change :)
>
> > > > That's why I suggested something like
> > > > >
> > > > >   #define fieldno_size   2
> > > > >   #define fieldno_korr   uint2korr
> > > > >   #define fieldno_store  int2store
> > > > >
> > > > Oh no, that'll add 9 new macros! And the number can grow...
> > >
> > > What do you mean, it's just three new macros, like above. Where's 9?
> >
> > 3 for fieldno, 3 for field_name_len, 3 for key_count (WITHOUT OVERLAPS
> > commit), and who knows -- I've also planned an flag in FOREIGN KEY
> support.
>
> sure, they're different entities, there is no inherent rule why they all
> should use the same storage width. So yes, 9 macros.

Seems, the consensual solution is just to add size consts, one per category.
I've updated the code. Are You good with that?
-- 
Yours truly,
Nikita Malyavin

References