Constraint documentation

Started by Lætitia Avrotover 7 years ago32 messages
#1Lætitia Avrot
laetitia.avrot@gmail.com
1 attachment(s)

Hi,

Patrick Francelle and I encountered this situation where there was a check
constraint on a table using a function to enforce a constraint across two
different tables. When using pg_dump to dump structure and data we found
out we couldn't restore it because tables weren't dumped in the right order
regarding that constraint.

Then, we found out this thread
/messages/by-id/11619.1077803699@sss.pgh.pa.us
where Tom explained how "check constraint were not intended to handle
cross-table checks" and how you should use a trigger instead. If you look
at modeling databases books, you'll actually find someting similar.

By looking at the constraint documentation page, we found out there was
nothing about it. So we decided to write a first version of a patch.

You will find it enclosed.

Here are some informations about it :
Project : postgresql
Branch : master
Applying, compilation and test : I applied it successfully. It compiles
sucessfully and I tested it on my laptop
Platform-specific : there shouldn't be any platform specific item
Regression tests : regression tests are not available for documentation
Documentation : We don't document documentation source code
Performance impact : none
Choices I made and why : I choose to include a trigger link to help users
go to the accurate documentation section. I also choose to add it as a note
so it's more visible (but I'm open minded on that matter)
Adresses a todo item : no

Please let me tell me know if I missed something. I'm waiting for feedbacks
to improve that patch.

Cheers,

Lætitia

--
*Think! Do you really need to print this email ? *
*There is no Planet B.*

Attachments:

check_constraint_accross_table_note_v1.patchtext/x-patch; charset=US-ASCII; name=check_constraint_accross_table_note_v1.patchDownload
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 2cd0b8a..dfe00a5 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -403,6 +403,17 @@ CREATE TABLE products (
     ensure that a column does not contain null values, the not-null
     constraint described in the next section can be used.
    </para>
+
+   <note>
+    <para>
+     Check constraint were not designed to enforce business rules across tables.
+     Avoid using check constraints with function accessing to other tables and
+     prefer triggers instead (please refer to <xref linkend="triggers"/> for
+     more information about triggers). PostgreSQL won't prevent you
+     from doing so, but be aware you might encounter difficulties to restore
+     dumps (generated with pg_dump or pg_dumpall) if you do.
+    </para>
+   </note>
   </sect2>
 
   <sect2>
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Lætitia Avrot (#1)
Re: Constraint documentation

[Message body could not be decoded - encoding: utf-8, error: Don't know how to decode utf-8, please call #encoded and decode it yourself.]

#3Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Lætitia Avrot (#1)
Re: Constraint documentation

Hello lætitia,

My 0.02 € to try to simplify the suggested documentation.

Check constraint were not

are not

designed to enforce business rules across tables.

Avoid using check constraints with function accessing to other tables

accessing other tables (no "to")

and prefer triggers instead (please refer to <xref linkend="triggers"/>
for more information about triggers).

... and use <xref linkend="triggers"/> instead.

PostgreSQL won't prevent you from doing so,

Although PostgreSQL ... so,

but be aware you might encounter difficulties to restore dumps
(generated with pg_dump or pg_dumpall) if you do.

beware that dumps generated by <application>pg_*<...> or <...> may be hard
to restore because of such checks, as the underlying dependencies are not
taken into account.

--
Fabien.

#4Lætitia Avrot
laetitia.avrot@gmail.com
In reply to: Fabien COELHO (#3)
Re: Constraint documentation

Thanks!
I'll correct the patch ASAP including your modifications.

Le sam. 23 juin 2018 à 19:15, Fabien COELHO <coelho@cri.ensmp.fr> a écrit :

Hello lætitia,

My 0.02 € to try to simplify the suggested documentation.

Check constraint were not

are not

designed to enforce business rules across tables.

Avoid using check constraints with function accessing to other tables

accessing other tables (no "to")

and prefer triggers instead (please refer to <xref linkend="triggers"/>
for more information about triggers).

... and use <xref linkend="triggers"/> instead.

PostgreSQL won't prevent you from doing so,

Although PostgreSQL ... so,

but be aware you might encounter difficulties to restore dumps
(generated with pg_dump or pg_dumpall) if you do.

beware that dumps generated by <application>pg_*<...> or <...> may be hard
to restore because of such checks, as the underlying dependencies are not
taken into account.

--
Fabien.

--
*Think! Do you really need to print this email ? *
*There is no Planet B.*

#5Lætitia Avrot
laetitia.avrot@gmail.com
In reply to: Lætitia Avrot (#4)
1 attachment(s)
Re: Constraint documentation

Hello,

Ok, I corrected the patch as suggested. I hope I did it right.

Have a nice day,

Lætitia

Le lun. 25 juin 2018 à 16:02, Lætitia Avrot <laetitia.avrot@gmail.com> a
écrit :

Thanks!
I'll correct the patch ASAP including your modifications.

Le sam. 23 juin 2018 à 19:15, Fabien COELHO <coelho@cri.ensmp.fr> a
écrit :

Hello lætitia,

My 0.02 € to try to simplify the suggested documentation.

Check constraint were not

are not

designed to enforce business rules across tables.

Avoid using check constraints with function accessing to other tables

accessing other tables (no "to")

and prefer triggers instead (please refer to <xref linkend="triggers"/>
for more information about triggers).

... and use <xref linkend="triggers"/> instead.

PostgreSQL won't prevent you from doing so,

Although PostgreSQL ... so,

but be aware you might encounter difficulties to restore dumps
(generated with pg_dump or pg_dumpall) if you do.

beware that dumps generated by <application>pg_*<...> or <...> may be
hard
to restore because of such checks, as the underlying dependencies are not
taken into account.

--
Fabien.

--
*Think! Do you really need to print this email ? *
*There is no Planet B.*

--
*Think! Do you really need to print this email ? *
*There is no Planet B.*

Attachments:

check_constraint_accross_table_note_v2.patchtext/x-patch; charset=US-ASCII; name=check_constraint_accross_table_note_v2.patchDownload
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 50dc25f..c677dc4 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -403,6 +403,18 @@ CREATE TABLE products (
     ensure that a column does not contain null values, the not-null
     constraint described in the next section can be used.
    </para>
+
+   <note>
+    <para>
+     Check constraint are not designed to enforce business rules across tables.
+     Avoid using check constraints with function accessing other tables and
+     use <xref linkend="triggers"/> instead. Although PostgreSQL won't prevent you
+     from doing so, but beware that dumps generated by <application>pg_dump</application>
+     or <application>pg_dumpall</application> may be hard
+     to restore because of such checks, as the underlying dependencies are not
+     taken into account.
+    </para>
+   </note>
   </sect2>
 
   <sect2>
#6Vik Fearing
vik.fearing@2ndquadrant.com
In reply to: Lætitia Avrot (#5)
Re: Constraint documentation

On 25/06/18 17:45, Lætitia Avrot wrote:

+   <note>
+    <para>
+     Check constraint are not designed to enforce business rules across tables.
+     Avoid using check constraints with function accessing other tables and

"with functions" or "with a function". I prefer the former.

+     use <xref linkend="triggers"/> instead. Although PostgreSQL won't prevent you
+     from doing so, but beware that dumps generated by <application>pg_dump</application>

No but.

+     or <application>pg_dumpall</application> may be hard
+     to restore because of such checks, as the underlying dependencies are not
+     taken into account.
+    </para>
+   </note>

--
Vik Fearing +33 6 46 75 15 36
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

#7Brad DeJong
bpd0018@gmail.com
In reply to: Vik Fearing (#6)
Re: Constraint documentation

On 25/06/18 17:45, Lætitia Avrot wrote:

+   <note>
+    <para>
+     Check constraint are not designed to enforce business rules across

tables.

+ Avoid using check constraints with function accessing other tables

and

Subject/verb agreement - either "A check constraint is ..." or "Check
constraints are ..." would be appropriate.

#8Lætitia Avrot
laetitia.avrot@gmail.com
In reply to: Brad DeJong (#7)
1 attachment(s)
Re: Constraint documentation

Hi,

Thanks a lot. I did the modification. It's in the patch enclosed.

Have a nice day,

Lætitia

Le mar. 26 juin 2018 à 01:42, Brad DeJong <bpd0018@gmail.com> a écrit :

On 25/06/18 17:45, Lætitia Avrot wrote:

+   <note>
+    <para>
+     Check constraint are not designed to enforce business rules across

tables.

+ Avoid using check constraints with function accessing other tables

and

Subject/verb agreement - either "A check constraint is ..." or "Check
constraints are ..." would be appropriate.

--
*Think! Do you really need to print this email ? *
*There is no Planet B.*

Attachments:

check_constraint_accross_table_note_v3.patchtext/x-patch; charset=US-ASCII; name=check_constraint_accross_table_note_v3.patchDownload
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 50dc25f..d7cc1d9 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -403,6 +403,18 @@ CREATE TABLE products (
     ensure that a column does not contain null values, the not-null
     constraint described in the next section can be used.
    </para>
+
+   <note>
+    <para>
+     Check constraints are not designed to enforce business rules across tables.
+     Avoid using check constraints with a function accessing other tables and
+     use <xref linkend="triggers"/> instead. Although PostgreSQL won't prevent you
+     from doing so, beware that dumps generated by <application>pg_dump</application>
+     or <application>pg_dumpall</application> may be hard
+     to restore because of such checks, as the underlying dependencies are not
+     taken into account.
+    </para>
+   </note>
   </sect2>
 
   <sect2>
#9Vik Fearing
vik.fearing@2ndquadrant.com
In reply to: Lætitia Avrot (#8)
Re: Constraint documentation

On 26/06/18 09:49, Lætitia Avrot wrote:

Thanks a lot. I did the modification. It's in the patch enclosed.

Looks good to me.
--
Vik Fearing +33 6 46 75 15 36
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

#10Brad DeJong
bpd0018@gmail.com
In reply to: Vik Fearing (#9)
Re: Constraint documentation

On Tue, Jun 26, 2018 at 4:24 AM, Vik Fearing <vik.fearing@2ndquadrant.com>
wrote:

Looks good to me.

I'll second that. Looks good to me too.

#11Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Lætitia Avrot (#8)
Re: Constraint documentation

On 6/26/18 09:49, Lætitia Avrot wrote:

+   <note>
+    <para>
+     Check constraints are not designed to enforce business rules across tables.
+     Avoid using check constraints with a function accessing other tables and
+     use <xref linkend="triggers"/> instead. Although PostgreSQL won't prevent you
+     from doing so, beware that dumps generated by <application>pg_dump</application>
+     or <application>pg_dumpall</application> may be hard
+     to restore because of such checks, as the underlying dependencies are not
+     taken into account.
+    </para>
+   </note>

In a way, I think this is attacking the wrong problem. It is saying
that you should use triggers instead of check constraints in certain
circumstances. But triggers are also used as an implementation detail
of constraints. While it currently doesn't exist, a deferrable check
constraint would probably be implemented as a trigger. It's not the
triggerness that fixes this problem. The problem is more generally that
if a function uses a table, then pg_dump can't know about the ordering.
It happens to work for triggers because triggers are dumped after all
tables, as a performance optimization, and we could very well dump check
constraints differently as well.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#12Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Peter Eisentraut (#11)
Re: Constraint documentation

Hello Peter,

+   <note>
+    <para>
+     Check constraints are not designed to enforce business rules across tables.
+     Avoid using check constraints with a function accessing other tables and
+     use <xref linkend="triggers"/> instead. Although PostgreSQL won't prevent you
+     from doing so, beware that dumps generated by <application>pg_dump</application>
+     or <application>pg_dumpall</application> may be hard
+     to restore because of such checks, as the underlying dependencies are not
+     taken into account.
+    </para>
+   </note>

In a way, I think this is attacking the wrong problem. It is saying
that you should use triggers instead of check constraints in certain
circumstances.

Yep. The circumstance is being able to dump & restore the database. If you
do not care about dump/restore, check constraints are always fine.

But triggers are also used as an implementation detail of constraints.

Yep.

It's not the triggerness that fixes this problem. The problem is more
generally that if a function uses a table, then pg_dump can't know about
the ordering.

Yep.

It happens to work for triggers because triggers are dumped after all
tables, as a performance optimization, and we could very well dump check
constraints differently as well.

Sure.

ISTM that is more or less what the text is saying?

I'm not sure what is the suggestion wrt to the documentation text. Is the
issue only with the first introductory sentence? Would removing it be
enough?

--
Fabien.

#13Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Fabien COELHO (#12)
Re: Constraint documentation

On 07.07.18 10:23, Fabien COELHO wrote:

I'm not sure what is the suggestion wrt to the documentation text. Is the
issue only with the first introductory sentence? Would removing it be
enough?

Yes. But it would be even better to fix pg_dump.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#14Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Peter Eisentraut (#13)
Re: Constraint documentation

Hello Peter,

I'm not sure what is the suggestion wrt to the documentation text. Is the
issue only with the first introductory sentence? Would removing it be
enough?

Yes. But it would be even better to fix pg_dump.

Sure. The purpose of Lætitia patch is simply to document the consequences
if the current behavior. Fixing pg_dump is another issue:-)

I guess that this would involve postponing all non trivial CHECK
declarations to after all table and function creations. While waiting for
such a significant change, ISTM that documenting the issue is a reasonable
option.

--
Fabien.

#15Lætitia Avrot
laetitia.avrot@gmail.com
In reply to: Peter Eisentraut (#11)
Re: Constraint documentation

Hi Peter,

I understand what you're pointing at and I agree that it could be a good
thing to be able to dump/restore a table without problem.

My point was that check constraints weren't supposed to be used that way
theorically (or maybe i'm mistaken ?) so I thought maybe we should just
inform the user that this kind of use of a check constraint is a misuse of
that feature.

Maybe it's not the right way to say it. I can remove the part about pg_dump
if it's too confusing...

Regards,

Lætitia

Le mer. 27 juin 2018 à 08:44, Peter Eisentraut <
peter.eisentraut@2ndquadrant.com> a écrit :

On 6/26/18 09:49, Lætitia Avrot wrote:

+   <note>
+    <para>
+     Check constraints are not designed to enforce business rules

across tables.

+ Avoid using check constraints with a function accessing other

tables and

+ use <xref linkend="triggers"/> instead. Although PostgreSQL won't

prevent you

+ from doing so, beware that dumps generated by

<application>pg_dump</application>

+     or <application>pg_dumpall</application> may be hard
+     to restore because of such checks, as the underlying dependencies

are not

+     taken into account.
+    </para>
+   </note>

In a way, I think this is attacking the wrong problem. It is saying
that you should use triggers instead of check constraints in certain
circumstances. But triggers are also used as an implementation detail
of constraints. While it currently doesn't exist, a deferrable check
constraint would probably be implemented as a trigger. It's not the
triggerness that fixes this problem. The problem is more generally that
if a function uses a table, then pg_dump can't know about the ordering.
It happens to work for triggers because triggers are dumped after all
tables, as a performance optimization, and we could very well dump check
constraints differently as well.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
*Think! Do you really need to print this email ? *
*There is no Planet B.*

#16Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Lætitia Avrot (#15)
Re: Constraint documentation

On 2018-Aug-07, L�titia Avrot wrote:

Hi Peter,

I understand what you're pointing at and I agree that it could be a good
thing to be able to dump/restore a table without problem.

My point was that check constraints weren't supposed to be used that way
theorically (or maybe i'm mistaken ?) so I thought maybe we should just
inform the user that this kind of use of a check constraint is a misuse of
that feature.

Tom Lane pointed out in another thread that the SQL standard lists
feature F673 "Reads SQL-data routine invocations in CHECK constraints"
which permits CHECK constraints to examine tables, so saying "you're not
supposed to do this", while correct from a Postgres perspective, would
be short-sighted ISTM, because we will make ourselves liars as soon as
we implement the feature.

I agree that we should point this out in *some* way, just not sure how.
Maybe something like "Postgres does not currently support CHECK
constraints containing queries, therefore we recommend to avoid them."
I would not mention pg_dump by name, just say dumps may not restore
depending on phase of moon.

(BTW I'm not sure of the term "other tables". You could have a query
that references the same table ...)

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#17Pantelis Theodosiou
ypercube@gmail.com
In reply to: Alvaro Herrera (#16)
Re: Constraint documentation

On Thu, Aug 9, 2018 at 10:32 PM, Alvaro Herrera <alvherre@2ndquadrant.com>
wrote:

On 2018-Aug-07, Lætitia Avrot wrote:

Hi Peter,

I understand what you're pointing at and I agree that it could be a good
thing to be able to dump/restore a table without problem.

My point was that check constraints weren't supposed to be used that way
theorically (or maybe i'm mistaken ?) so I thought maybe we should just
inform the user that this kind of use of a check constraint is a misuse

of

that feature.

Tom Lane pointed out in another thread that the SQL standard lists
feature F673 "Reads SQL-data routine invocations in CHECK constraints"
which permits CHECK constraints to examine tables, so saying "you're not
supposed to do this", while correct from a Postgres perspective, would
be short-sighted ISTM, because we will make ourselves liars as soon as
we implement the feature.

I agree that we should point this out in *some* way, just not sure how.
Maybe something like "Postgres does not currently support CHECK
constraints containing queries, therefore we recommend to avoid them."
I would not mention pg_dump by name, just say dumps may not restore
depending on phase of moon.

(BTW I'm not sure of the term "other tables". You could have a query
that references the same table ...)

--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

I like this:

"Postgres does not currently support CHECK constraints containing

queries, therefore we recommend to avoid them."

Perhaps adding:

CHECK constraints are currently meant to be used as *row constraints*

only.

Use - if possible - UNIQUE or EXCLUDE constraints. for constraints that

involve many or all rows of a table,

and FOREIGN KEY constraints for cross table constraints.
More complex constraints will be available when ASSERTION are implemented.

And then adding some warning about using functions in CHECK constraints to
bypass current limitations.

Pantelis Theodsoiou

#18Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Alvaro Herrera (#16)
Re: Constraint documentation

On 09/08/2018 23:32, Alvaro Herrera wrote:

I agree that we should point this out in *some* way, just not sure how.
Maybe something like "Postgres does not currently support CHECK
constraints containing queries, therefore we recommend to avoid them."
I would not mention pg_dump by name, just say dumps may not restore
depending on phase of moon.

I think it would be very easy to restore check constraints separately
after all tables in pg_dump. There is already support for that, but
it's only used when necessary, for things like not-valid constraints.
The argument in favor of keeping the constraint with the table is
probably only aesthetics, but of course the argument against is that it
sometimes doesn't work. So we could either enhance the smarts about
when to use the "dump separately" path (this might be difficult), or
just use it always.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#19Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#18)
Re: Constraint documentation

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

I think it would be very easy to restore check constraints separately
after all tables in pg_dump. There is already support for that, but
it's only used when necessary, for things like not-valid constraints.
The argument in favor of keeping the constraint with the table is
probably only aesthetics,

No, it's mainly about performance. Checking the constraint at data load
time avoids extra scans of the table, and should work in any case that
we consider supported.

To be clear, I totally reject the notion that we should consider this
case supported, or that kluging pg_dump to not fail would make it so.
As a counterexample, if you have a poor-mans-FK check constraint on
table A that only succeeds when there's a matching row in table B, it
cannot prevent the case where you insert a valid (matching) row in
table A and then later delete its matching row in B.

Maybe someday we'll have full database assertions (with, no doubt,
a ton of performance caveats). In the meantime, let's not slow down
CHECK constraints for everyone in order to partially fix a
fundamentally broken use-case. If the documentation isn't clear enough
about such cases being unsupported, by all means let's make it so.

regards, tom lane

#20David Fetter
david@fetter.org
In reply to: Peter Eisentraut (#18)
Re: Constraint documentation

On Fri, Aug 10, 2018 at 12:27:49PM +0200, Peter Eisentraut wrote:

On 09/08/2018 23:32, Alvaro Herrera wrote:

I agree that we should point this out in *some* way, just not sure how.
Maybe something like "Postgres does not currently support CHECK
constraints containing queries, therefore we recommend to avoid them."
I would not mention pg_dump by name, just say dumps may not restore
depending on phase of moon.

I think it would be very easy to restore check constraints separately
after all tables in pg_dump. There is already support for that, but
it's only used when necessary, for things like not-valid constraints.
The argument in favor of keeping the constraint with the table is
probably only aesthetics, but of course the argument against is that it
sometimes doesn't work. So we could either enhance the smarts about
when to use the "dump separately" path (this might be difficult), or
just use it always.

+1 for dumping all constraints separately by default.

Currently, it's possible to create unrestorable databases without
fiddling with the catalog, as a legacy database I was dealing with
just last week demonstrated.

It occurs to me that the aesthetic issues might be dealt with by
having a separate "aesthetic" restore mode, which is to say what you'd
expect if you were writing the schema code /de novo/. This would be
non-trivial to get right in some cases, and flat-out impossible for
cases where we can't see into the code that could produce a
dependency.

Best,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

#21David Fetter
david@fetter.org
In reply to: Tom Lane (#19)
Re: Constraint documentation

On Fri, Aug 10, 2018 at 09:47:09AM -0400, Tom Lane wrote:

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

I think it would be very easy to restore check constraints separately
after all tables in pg_dump. There is already support for that, but
it's only used when necessary, for things like not-valid constraints.
The argument in favor of keeping the constraint with the table is
probably only aesthetics,

No, it's mainly about performance. Checking the constraint at data load
time avoids extra scans of the table, and should work in any case that
we consider supported.

We could deal with this by putting those constraints in the "pre-data"
section, which would let people do any needed surgery using the
standard pg_restore -l/-L machinery, should they actually happen to be
"post-data" constraints.

To be clear, I totally reject the notion that we should consider this
case supported, or that kluging pg_dump to not fail would make it so.
As a counterexample, if you have a poor-mans-FK check constraint on
table A that only succeeds when there's a matching row in table B, it
cannot prevent the case where you insert a valid (matching) row in
table A and then later delete its matching row in B.

That's the case I ran into last week, and it required a schema change
in order to ensure that dumps were restorable in their unmodified
form, that being crucial to disaster recovery operations.

Maybe someday we'll have full database assertions (with, no doubt,
a ton of performance caveats).

The initial performance will likely be pretty awful for isolation
levels lower than SERIALIZABLE, anyhow.

Best,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

#22Andres Freund
andres@anarazel.de
In reply to: Tom Lane (#19)
Re: Constraint documentation

On August 10, 2018 7:17:09 PM GMT+05:30, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

I think it would be very easy to restore check constraints separately
after all tables in pg_dump. There is already support for that, but
it's only used when necessary, for things like not-valid constraints.
The argument in favor of keeping the constraint with the table is
probably only aesthetics,

No, it's mainly about performance. Checking the constraint at data
load
time avoids extra scans of the table, and should work in any case that
we consider supported.

To be clear, I totally reject the notion that we should consider this
case supported, or that kluging pg_dump to not fail would make it so.
As a counterexample, if you have a poor-mans-FK check constraint on
table A that only succeeds when there's a matching row in table B, it
cannot prevent the case where you insert a valid (matching) row in
table A and then later delete its matching row in B.

Maybe someday we'll have full database assertions (with, no doubt,
a ton of performance caveats). In the meantime, let's not slow down
CHECK constraints for everyone in order to partially fix a
fundamentally broken use-case. If the documentation isn't clear enough
about such cases being unsupported, by all means let's make it so.

+1

--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

#23Patrick Francelle
patrick@francelle.name
In reply to: Pantelis Theodosiou (#17)
1 attachment(s)
Re: Constraint documentation

On 08/10/18 07:02, Pantelis Theodosiou wrote:

I like this:

"Postgres does not currently support CHECK constraints containing

queries, therefore we recommend to avoid them."

Perhaps adding:

CHECK constraints are currently meant to be used as *row constraints*

only.

Use - if possible - UNIQUE or EXCLUDE constraints. for constraints

that involve many or all rows of a table,

and FOREIGN KEY constraints for cross table constraints.
More complex constraints will be available when ASSERTION are implemented.

And then adding some warning about using functions in CHECK constraints
to bypass current limitations.

Pantelis Theodsoiou

Hi,

I have rewritten the patch to include your remarks.
In fact, the whole patch is now based on it, so thank you.

Patrick Francelle

Attachments:

check_constraint_accross_table_note_v4.patchtext/x-patch; name=check_constraint_accross_table_note_v4.patchDownload
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index b5ed1b7939..f57f6e716d 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -403,6 +403,18 @@ CREATE TABLE products (
     ensure that a column does not contain null values, the not-null
     constraint described in the next section can be used.
    </para>
+
+   <note>
+    <para>
+     Postgres does not currently support CHECK constraints containing queries,
+     therefore we recommend to avoid them.  By using them, you may expect
+     troubles, especially to restore dumps.
+     CHECK constraints are currently meant to be used as row constraints only.
+     Use - if possible - UNIQUE or EXCLUDE constraints for constraints that
+     involve many or all rows of a table, and FOREIGN KEY constraints for cross
+     table constraints.
+    </para>
+   </note>
   </sect2>
 
   <sect2>
#24David G. Johnston
david.g.johnston@gmail.com
In reply to: Patrick Francelle (#23)
Re: Constraint documentation

The product name, when used in the documentation, is "PostgreSQL" with
appropriate html elements surrounding it.

Some parts that look or read oddly to me:
"you may expect troubles"
Use - if possible - (commas, not hypens, are customary here)
"does not currently" - drop "currently", it doesn't and we don't need to
predict the future (same goes for "are currently meant")
"therefore we recommend to avoid them" - they are unsupported, the implied
recommended is to not use them period, not avoid them if possible. Better
to say that it isn't enforced even though it is unsupported.

An alternative to consider as one the whole the reading of the v4 patch
just feels off and different than the rest of that section of the
documentation.

PostgreSQL does not support writing CHECK constraints that reference tables
(though it does not reliably prevent one from doing so). While normal
operations are likely to succeed even if you violate this rule it is
probable that a database restoration will fail. Use FOREIGN KEY
constraints or custom triggers for cross-table validations. For rows on
the same table you should use UNIQUE or EXCLUDE constraints when
applicable, or a custom trigger otherwise.

David J.

On Tue, Oct 30, 2018 at 8:50 AM Patrick Francelle <patrick@francelle.name>
wrote:

Show quoted text

On 08/10/18 07:02, Pantelis Theodosiou wrote:

I like this:

"Postgres does not currently support CHECK constraints containing

queries, therefore we recommend to avoid them."

Perhaps adding:

CHECK constraints are currently meant to be used as *row constraints*

only.

Use - if possible - UNIQUE or EXCLUDE constraints. for constraints

that involve many or all rows of a table,

and FOREIGN KEY constraints for cross table constraints.
More complex constraints will be available when ASSERTION are

implemented.

And then adding some warning about using functions in CHECK constraints
to bypass current limitations.

Pantelis Theodsoiou

Hi,

I have rewritten the patch to include your remarks.
In fact, the whole patch is now based on it, so thank you.

Patrick Francelle

#25Patrick Francelle
patrick@francelle.name
In reply to: David G. Johnston (#24)
1 attachment(s)
Re: Constraint documentation

Thanks for your remarks and advices, and of course for your help to
rewrite the text.
So, it is now included in the new version attached.
I hope it will be ok this time.

Patrick Francelle

Show quoted text

On 10/30/18 17:14, David G. Johnston wrote:

The product name, when used in the documentation, is "PostgreSQL" with
appropriate html elements surrounding it.

Some parts that look or read oddly to me:
"you may expect troubles"
Use - if possible - (commas, not hypens, are customary here)
"does not currently" - drop "currently", it doesn't and we don't need
to predict the future (same goes for "are currently meant")
"therefore we recommend to avoid them" - they are unsupported, the
implied recommended is to not use them period, not avoid them if
possible.  Better to say that it isn't enforced even though it is
unsupported.

An alternative to consider as one the whole the reading of the v4
patch just feels off and different than the rest of that section of
the documentation.

PostgreSQL does not support writing CHECK constraints that reference
tables (though it does not reliably prevent one from doing so).  While
normal operations are likely to succeed even if you violate this rule
it is probable that a database restoration will fail.  Use FOREIGN KEY
constraints or custom triggers for cross-table validations.  For rows
on the same table you should use UNIQUE or EXCLUDE constraints when
applicable, or a custom trigger otherwise.

David J.

Attachments:

check_constraint_accross_table_note_v5.patchtext/x-patch; name=check_constraint_accross_table_note_v5.patchDownload
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index b5ed1b7939..142918e2b1 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -403,6 +403,20 @@ CREATE TABLE products (
     ensure that a column does not contain null values, the not-null
     constraint described in the next section can be used.
    </para>
+
+   <note>
+    <para>
+     <productname>PostgreSQL</productname> does not support writing
+     <literal>CHECK</literal> constraints that reference tables (though it does
+     not reliably prevent one from doing so).  While normal operations are
+     likely to succeed even if you violate this rule, it is probable that a
+     database restoration will fail.  Use <literal>FOREIGN KEY</literal>
+     constraints or custom <link linkend="triggers">triggers</link> for
+     cross-table validations.  For rows on the same table you should use
+     <literal>UNIQUE</literal> or <literal>EXCLUDE</literal> constraints when
+     applicable, or a custom trigger otherwise.
+    </para>
+   </note>
   </sect2>
 
   <sect2>
#26Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Patrick Francelle (#25)
Re: Constraint documentation

Thanks for your remarks and advices, and of course for your help to
rewrite the text.
So, it is now included in the new version attached.
I hope it will be ok this time.

At least it looks ok to me.

Patch applies cleanly, doc build ok.

I've put the patch as "Ready".

--
Fabien.

#27Tom Lane
tgl@sss.pgh.pa.us
In reply to: Fabien COELHO (#26)
Re: Constraint documentation

Fabien COELHO <coelho@cri.ensmp.fr> writes:

I've put the patch as "Ready".

I think this could be improved some more. Perhaps something like this
(I've not bothered with markup...)

PostgreSQL does not support CHECK constraints that reference table
data other than the new or updated row being checked. While a CHECK
constraint that violates this rule may appear to work in simple
tests, it cannot guarantee that the database will not reach a state
in which the constraint condition is false (due to subsequent changes
of the other row(s) involved). This would cause a database dump and
reload to fail. The reload could fail even when the complete
database state is consistent with the constraint, due to rows not
being loaded in an order that will satisfy the constraint. If
possible, use UNIQUE, EXCLUDE, or FOREIGN KEY constraints to express
cross-row and cross-table restrictions.

If what you desire is a one-time check against other rows at row
insertion, rather than a continuously-maintained consistency
guarantee, a custom trigger can be used to implement that. (This
approach avoids the dump/reload problem because pg_dump does not
reinstall triggers until after reloading data, so that the check will
not be enforced during a dump/reload.)

This is a little verbose maybe, but as the text stands, it sounds like
using a trigger is enough to solve all the consistency problems that
a cross-row CHECK has. Which it's not of course.

I'm also wondering whether it's better to put this in the CREATE TABLE
reference page instead of here. While there are certainly benefits in
having the caveat here, I'm a bit troubled by the number of forward
references to concepts that are described later. OTOH, a lot of people
who need the warning might never see it if it's buried in the reference
material.

regards, tom lane

#28Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Tom Lane (#27)
Re: Constraint documentation

I set this patch waiting-on-author. Please update.

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#29Patrick Francelle
patrick@francelle.name
In reply to: Tom Lane (#27)
1 attachment(s)

On 11/15/18 00:02, Tom Lane wrote:

I think this could be improved some more. Perhaps something like this
(I've not bothered with markup...)

This is a little verbose maybe, but as the text stands, it sounds like
using a trigger is enough to solve all the consistency problems that
a cross-row CHECK has. Which it's not of course.

Thank you for the rewriting, this is much more clear and explicit that way.

I'm also wondering whether it's better to put this in the CREATE TABLE
reference page instead of here. While there are certainly benefits in
having the caveat here, I'm a bit troubled by the number of forward
references to concepts that are described later. OTOH, a lot of people
who need the warning might never see it if it's buried in the reference
material.

To address your remark, I added a small message in the CREATE TABLE
reference page to be more explicit about the topic, so that it would be
a warning for the users reading the section. And then a reference to the
CHECK constraint page where the full explanation is to be located.

That way, the caveat is mentioned in both pages, but the full
explanation is located only on a single page.

Please, let me know if this is good enough or maybe if I missed
something.

Patrick Francelle

Attachments:

check_constraint_accross_table_note_v6.patchtext/x-patch; name=check_constraint_accross_table_note_v6.patchDownload
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 61c4a25460..bfe89ef8ae 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -403,6 +403,33 @@ CREATE TABLE products (
     ensure that a column does not contain null values, the not-null
     constraint described in the next section can be used.
    </para>
+
+   <note>
+    <para>
+     <productname>PostgreSQL</productname> does not support
+     <literal>CHECK</literal> constraints that reference table data other than
+     the new or updated row being checked.  While a <literal>CHECK</literal>
+     constraint that violates this rule may appear to work in simple
+     tests, it cannot guarantee that the database will not reach a state
+     in which the constraint condition is false (due to subsequent changes
+     of the other row(s) involved).  This would cause a database dump and
+     reload to fail.  The reload could fail even when the complete
+     database state is consistent with the constraint, due to rows not
+     being loaded in an order that will satisfy the constraint.  If
+     possible, use <literal>UNIQUE</literal>, <literal>EXCLUDE</literal>,
+     or <literal>FOREIGN KEY</literal> constraints to express
+     cross-row and cross-table restrictions.
+    </para>
+
+    <para>
+     If what you desire is a one-time check against other rows at row
+     insertion, rather than a continuously-maintained consistency
+     guarantee, a custom <link linkend="triggers">trigger</link> can be used
+     to implement that.  (This approach avoids the dump/reload problem because
+     <application>pg_dump</application> does not reinstall triggers until after
+     reloading data, so that the check will not be enforced during a dump/reload.)
+    </para>
+   </note>
   </sect2>
 
   <sect2>
diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml
index 50d5597002..d6d1191ddd 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -756,6 +756,8 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
       subqueries nor refer to variables other than columns of the
       current row.  The system column <literal>tableoid</literal>
       may be referenced, but not any other system column.
+      Also, <literal>CHECK</literal> constraints that references other tables
+      are not supported (see <xref linkend="ddl-constraints-check-constraints"/>).
      </para>
 
      <para>

#30David Fetter
david@fetter.org
In reply to: Patrick Francelle (#29)
Re: Constraint documentation

On Thu, Nov 22, 2018 at 03:16:11PM +0100, Patrick Francelle wrote:

On 11/15/18 00:02, Tom Lane wrote:

I think this could be improved some more. Perhaps something like this
(I've not bothered with markup...)

This is a little verbose maybe, but as the text stands, it sounds like
using a trigger is enough to solve all the consistency problems that
a cross-row CHECK has. Which it's not of course.

Thank you for the rewriting, this is much more clear and explicit that way.

I'm also wondering whether it's better to put this in the CREATE TABLE
reference page instead of here. While there are certainly benefits in
having the caveat here, I'm a bit troubled by the number of forward
references to concepts that are described later. OTOH, a lot of people
who need the warning might never see it if it's buried in the reference
material.

To address your remark, I added a small message in the CREATE TABLE
reference page to be more explicit about the topic, so that it would be
a warning for the users reading the section. And then a reference to the
CHECK constraint page where the full explanation is to be located.

That way, the caveat is mentioned in both pages, but the full
explanation is located only on a single page.

Please, let me know if this is good enough or maybe if I missed
something.

Patrick Francelle

I believe that features F671 (subqueries in CHECK constraints) and
possibly F673 (reads SQL-data routine invocations in CHECK
constraints) from the standard should be referred to here.

We haven't implemented either one of them, but we might some day.

Best,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

#31Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: David Fetter (#30)
Re: Constraint documentation

I have pushed this.

On 2018-Nov-26, David Fetter wrote:

On Thu, Nov 22, 2018 at 03:16:11PM +0100, Patrick Francelle wrote:

To address your remark, I added a small message in the CREATE TABLE
reference page to be more explicit about the topic, so that it would be
a warning for the users reading the section. And then a reference to the
CHECK constraint page where the full explanation is to be located.

That way, the caveat is mentioned in both pages, but the full
explanation is located only on a single page.

That was a good idea, but your third sentence repeated what was being
said in the first sentence in the same paragraph. I edited that to put
the cross-reference next to the first sentence instead.

I believe that features F671 (subqueries in CHECK constraints) and
possibly F673 (reads SQL-data routine invocations in CHECK
constraints) from the standard should be referred to here.

We haven't implemented either one of them, but we might some day.

I don't necessarily disagree, but I don't think we put many feature
references in the docs. I suppose we can edit it when we implement
F671 and F673. Or maybe you want to submit a followup patch. It didn't
seem worth blocking this patch for your proposed change (particularly
since L�titia seems to have given up on it already).

Thanks,

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#32Lætitia Avrot
laetitia.avrot@gmail.com
In reply to: Alvaro Herrera (#31)
Re: Constraint documentation

Hi all,

Thank you for helping us to make that patch better.

It didn't

seem worth blocking this patch for your proposed change (particularly
since Lætitia seems to have given up on it already).

I haven't given up. As I said in the begining, Patrick and I were working
together on that patch. As he was doing well, I let him do the job and
focused on something else. The patch as it's now seems pretty good to me.
I agree working on F671 and F673 would be great. Sadly, I need to grow as a
developer to be able to do that.

Cheers,

Lætitia

--
*Think! Do you really need to print this email ? *
*There is no Planet B.*