Lists: | pgsql-hackers |
---|
From: | Peter Eisentraut <peter_e(at)gmx(dot)net> |
---|---|
To: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | CHECK NO INHERIT syntax |
Date: | 2012-07-18 21:49:37 |
Message-ID: | 1342648177.31327.10.camel@vanquo.pezone.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Sorry to raise this once again, but I still find this CHECK NO INHERIT
syntax to a bit funny. We are currently using something like
CHECK NO INHERIT (foo > 0)
But we already have a different syntax for attaching attributes to
constraints (NOT DEFERRABLE, NOT VALID, etc.), so it would make more
sense to have
CHECK (foo > 0) NO INHERIT
Besides consistency, this makes more sense, because the attribute is a
property of the constraint as a whole, not of the "checking".
This would also extend more easily to other constraint types. For
example, when unifying CHECK and NOT NULL constraints, as is planned, or
when allowing inherited unique constraints, as is planned further down
the road.
There is also a hole in the current implementation. Domain constraints
silently allow NO INHERIT to be specified, even though other senseless
attributes are rejected.
From: | Robert Haas <robertmhaas(at)gmail(dot)com> |
---|---|
To: | Peter Eisentraut <peter_e(at)gmx(dot)net> |
Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: CHECK NO INHERIT syntax |
Date: | 2012-07-19 13:40:56 |
Message-ID: | CA+TgmobE9n2qwzPN3nPpOK-901_Ue+P9iNAPM2wVHqWEO1wOQg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On Wed, Jul 18, 2012 at 5:49 PM, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
> Sorry to raise this once again, but I still find this CHECK NO INHERIT
> syntax to a bit funny. We are currently using something like
>
> CHECK NO INHERIT (foo > 0)
>
> But we already have a different syntax for attaching attributes to
> constraints (NOT DEFERRABLE, NOT VALID, etc.), so it would make more
> sense to have
>
> CHECK (foo > 0) NO INHERIT
>
> Besides consistency, this makes more sense, because the attribute is a
> property of the constraint as a whole, not of the "checking".
>
> This would also extend more easily to other constraint types. For
> example, when unifying CHECK and NOT NULL constraints, as is planned, or
> when allowing inherited unique constraints, as is planned further down
> the road.
+1.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
From: | David Fetter <david(at)fetter(dot)org> |
---|---|
To: | Peter Eisentraut <peter_e(at)gmx(dot)net> |
Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: CHECK NO INHERIT syntax |
Date: | 2012-07-19 22:30:49 |
Message-ID: | 20120719223049.GA1981@fetter.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On Thu, Jul 19, 2012 at 12:49:37AM +0300, Peter Eisentraut wrote:
> Sorry to raise this once again, but I still find this CHECK NO INHERIT
> syntax to a bit funny. We are currently using something like
>
> CHECK NO INHERIT (foo > 0)
>
> But we already have a different syntax for attaching attributes to
> constraints (NOT DEFERRABLE, NOT VALID, etc.), so it would make more
> sense to have
>
> CHECK (foo > 0) NO INHERIT
How about this?
CHECK (foo > 0) (INHERIT FALSE)
That leaves an obvious place for other options, which will doubtless
come. EXPLAIN's options inspired this API design.
> Besides consistency, this makes more sense, because the attribute is a
> property of the constraint as a whole, not of the "checking".
Good point. The above change preserves this property.
> This would also extend more easily to other constraint types. For
> example, when unifying CHECK and NOT NULL constraints, as is
> planned, or when allowing inherited unique constraints, as is
> planned further down the road.
>
> There is also a hole in the current implementation. Domain
> constraints silently allow NO INHERIT to be specified, even though
> other senseless attributes are rejected.
That's probably a bug.
Cheers,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | David Fetter <david(at)fetter(dot)org> |
Cc: | Peter Eisentraut <peter_e(at)gmx(dot)net>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: CHECK NO INHERIT syntax |
Date: | 2012-07-20 00:05:41 |
Message-ID: | 19361.1342742741@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
David Fetter <david(at)fetter(dot)org> writes:
> On Thu, Jul 19, 2012 at 12:49:37AM +0300, Peter Eisentraut wrote:
>> But we already have a different syntax for attaching attributes to
>> constraints (NOT DEFERRABLE, NOT VALID, etc.), so it would make more
>> sense to have
>>
>> CHECK (foo > 0) NO INHERIT
> How about this?
> CHECK (foo > 0) (INHERIT FALSE)
The SQL spec already says what the syntax is for options attached to
constraints, and that's not it.
regards, tom lane
From: | Alvaro Herrera <alvherre(at)commandprompt(dot)com> |
---|---|
To: | Peter Eisentraut <peter_e(at)gmx(dot)net> |
Cc: | Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: CHECK NO INHERIT syntax |
Date: | 2012-07-20 20:07:13 |
Message-ID: | 1342814413-sup-2460@alvh.no-ip.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Excerpts from Peter Eisentraut's message of mié jul 18 17:49:37 -0400 2012:
> Sorry to raise this once again, but I still find this CHECK NO INHERIT
> syntax to a bit funny. We are currently using something like
>
> CHECK NO INHERIT (foo > 0)
>
> But we already have a different syntax for attaching attributes to
> constraints (NOT DEFERRABLE, NOT VALID, etc.), so it would make more
> sense to have
>
> CHECK (foo > 0) NO INHERIT
Okay, given the astounding acceptance of your proposal, the attached patch
fixes things in that way. This only include changes to the core code;
I'll prepare documentation and regression tests tweaks while I wait for an
answer to the request below.
> There is also a hole in the current implementation. Domain constraints
> silently allow NO INHERIT to be specified, even though other senseless
> attributes are rejected.
True. I have added an error check at creation time. Please suggest
improved wording for the message:
alvherre=# create domain positiveint2 as int check (value > 0) no inherit;
ERROR: CHECK constraints for domains cannot be NO INHERIT
--
Álvaro Herrera <alvherre(at)commandprompt(dot)com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Attachment | Content-Type | Size |
---|---|---|
check-no-inherit.patch | application/octet-stream | 2.5 KB |
From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Alvaro Herrera <alvherre(at)commandprompt(dot)com> |
Cc: | Peter Eisentraut <peter_e(at)gmx(dot)net>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: CHECK NO INHERIT syntax |
Date: | 2012-07-20 20:12:05 |
Message-ID: | 395.1342815125@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> True. I have added an error check at creation time. Please suggest
> improved wording for the message:
> alvherre=# create domain positiveint2 as int check (value > 0) no inherit;
> ERROR: CHECK constraints for domains cannot be NO INHERIT
I think "CHECK constraints for domains cannot be marked NO INHERIT"
would be fine.
> ConstraintElem:
> - CHECK opt_no_inherit '(' a_expr ')' ConstraintAttributeSpec
> + CHECK '(' a_expr ')' opt_no_inherit ConstraintAttributeSpec
This doesn't seem to me to meet the principle of least surprise. Surely
NO INHERIT ought to be folded into ConstraintAttributeSpec so that it
acts like other constraint decorations, ie order isn't significant.
regards, tom lane
From: | Alvaro Herrera <alvherre(at)commandprompt(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Peter Eisentraut <peter_e(at)gmx(dot)net>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: CHECK NO INHERIT syntax |
Date: | 2012-07-21 04:20:57 |
Message-ID: | 1342844177-sup-4470@alvh.no-ip.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Excerpts from Tom Lane's message of vie jul 20 16:12:05 -0400 2012:
>
> Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> > True. I have added an error check at creation time. Please suggest
> > improved wording for the message:
>
> > alvherre=# create domain positiveint2 as int check (value > 0) no inherit;
> > ERROR: CHECK constraints for domains cannot be NO INHERIT
>
> I think "CHECK constraints for domains cannot be marked NO INHERIT"
> would be fine.
Thanks.
> > ConstraintElem:
> > - CHECK opt_no_inherit '(' a_expr ')' ConstraintAttributeSpec
> > + CHECK '(' a_expr ')' opt_no_inherit ConstraintAttributeSpec
>
> This doesn't seem to me to meet the principle of least surprise. Surely
> NO INHERIT ought to be folded into ConstraintAttributeSpec so that it
> acts like other constraint decorations, ie order isn't significant.
Oh, true; that's a bit more involved. I verified it works correctly to
have a constraint marked NOT VALID NO INHERIT or the other way around.
I haven't checked whether the changes to ConstraintAttributeSpec have
side effects -- I think it's OK but I might be missing something.
Here's a (hopefully) complete patch.
--
Álvaro Herrera <alvherre(at)commandprompt(dot)com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Attachment | Content-Type | Size |
---|---|---|
check-no-inherit-2.patch | application/octet-stream | 25.2 KB |
From: | Alvaro Herrera <alvherre(at)commandprompt(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Peter Eisentraut <peter_e(at)gmx(dot)net>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: CHECK NO INHERIT syntax |
Date: | 2012-07-24 20:32:12 |
Message-ID: | 1343161796-sup-7296@alvh.no-ip.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Excerpts from Alvaro Herrera's message of sáb jul 21 00:20:57 -0400 2012:
> Here's a (hopefully) complete patch.
Pushed. I hope people like this one better (third time's the charm, and
all that).
--
Álvaro Herrera <alvherre(at)commandprompt(dot)com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support