maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Nikita!

On Feb 04, Nikita Malyavin wrote:
> On Fri, Jan 4, 2019 at 6:29 AM Sergei Golubchik <serg@xxxxxxxxxxx> wrote:
> 
> > > +insert t values(@e, @s);
> > > +ERROR 23000: CONSTRAINT `CONSTRAINT_1` failed for `test`.`t`
> >
> > Hmm. The standard, indeed, says "there's an implicit constraint".
> >
> > But it does not say "CHECK contraint". UNIQUE is also a constraint,
> > Foreign key is also a constraint.
> >
> > So, I think here it means really just *a* constraint (a limitation,
> > in other words), meaning only that start value must be before the
> > end value. It doesn't mean that a CHECK constraint must be created.
> >
> > In other words, it should not be a CHECK constraint, it should not
> > produce a `CONSTRAINT xxx failed` message, and, of course, it should
> > never ever be shown as a CHECK constraint.
> 
> 11.3 <table definition>, Syntax Rules, 2)e)v)2)B)
> 
>   Let S be the schema identified by the explicit or implicit <schema
>   name> of TN. Let IDCN be an implementation-dependent <constraint
>   name> that is not equivalent to the <constraint name> of any table
>   constraint descriptor included in S. The following <table constraint
>   definition> is implicit: CONSTRAINT IDCN CHECK ( CN1 < CN2 )

Good catch.

> And General Rules, 4) e)
> 
>   A table descriptor TDS is created that describes T. TDS includes:
> 
>   If a <table period definition> that contains an <application time period
>   specification> is specified that contains <application time period name>
>   ATPN, then a period descriptor that contains ATPN as the name of the
>   period, the names of the ATPN period start and ATPN period end columns, and
>   IDCN as the name of the implicit ATPN period constraint.
> 
> So it's CHECK constraint.
> I think that the standart is quite inconvenient with naming here, but note
> that it is implementation-dependant, so we might choose better naming rule
> later. Or explicitly violate the rule and just name the constraint with
> period name -- I already have such version, if you're interested.

Yes, I was thinking that making constraint name the same as a period
name would be good.

I'm still not sure that an "implicit constraint" means an actual
constraint is created and it's visible in SHOW CREATE TABLE.

Usually in such cases the standard says "the following statement is
effectively executed: INSERT ..." instead of "the followig INSERT is
implicit"

But saying "constraint XXX is violated" and not showing the constraint
XXX in the SHOW CREATE TABLE will be confusing as hell for every single
user. So let's add it explicitly, all right.

> > > diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy
> > > index 10535326ce5..545c8f831b9 100644
> > > --- a/sql/sql_yacc.yy
> > > +++ b/sql/sql_yacc.yy
> > > @@ -8190,6 +8195,10 @@ alter_list_item:
> > >            {
> > >              Lex->alter_info.flags|= ALTER_ADD_PERIOD;
> > >            }
> > > +        | ADD period_for_application_time
> >
> > where's ADD PERIOD if_not_exists?
> 
> Not very clean with the behavior expected. Consider CREATE TABLE t (s
> date, e date, period for a(s,e));
> Should ALTER TABLE ADD IF NOT EXISTS PERIOD FOR b replace PERIOD FOR a, or
> return ER_MORE_THAN_ONE_PERIOD?

ER_MORE_THAN_ONE_PERIOD of course. It adds a period name `b`, it's a
different period, and there can be only one. So - an error.

If it'd be

  ALTER TABLE ADD IF NOT EXISTS PERIOD FOR a(....)

it'd be a no-op.

I cannot see of a case when ALTER TABLE ADD IF NOT EXISTS should replace stuff.

Regards,
Sergei
Chief Architect MariaDB
and security@xxxxxxxxxxx