Constraint documentation

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
22 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Constraint documentation

Lætitia Avrot
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 https://www.postgresql.org/message-id/11619.1077803699%40sss.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.

check_constraint_accross_table_note_v1.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Constraint documentation

Tom Lane-2
=?UTF-8?Q?L=C3=A6titia_Avrot?= <[hidden email]> writes:
> ... 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.

Hi Lætitia!  Please add this thread to the open commitfest to make
sure we don't forget about it.  (The open -fest is dated 2018-09,
but it's likely we'll try to process some of it in July instead.)

                        regards, tom lane

Reply | Threaded
Open this post in threaded view
|

Re: Constraint documentation

Fabien COELHO-3
In reply to this post by Lætitia Avrot

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.
Reply | Threaded
Open this post in threaded view
|

Re: Constraint documentation

Lætitia Avrot
Thanks!
I'll correct the patch ASAP including your modifications.

Le sam. 23 juin 2018 à 19:15, Fabien COELHO <[hidden email]> 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.
Reply | Threaded
Open this post in threaded view
|

Re: Constraint documentation

Lætitia Avrot
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 <[hidden email]> a écrit :
Thanks!
I'll correct the patch ASAP including your modifications.

Le sam. 23 juin 2018 à 19:15, Fabien COELHO <[hidden email]> 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.

check_constraint_accross_table_note_v2.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Constraint documentation

Vik Fearing-4
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

Reply | Threaded
Open this post in threaded view
|

Re: Constraint documentation

Brad DeJong-2
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.
Reply | Threaded
Open this post in threaded view
|

Re: Constraint documentation

Lætitia Avrot
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 <[hidden email]> 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.

check_constraint_accross_table_note_v3.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Constraint documentation

Vik Fearing-4
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

Reply | Threaded
Open this post in threaded view
|

Re: Constraint documentation

Brad DeJong-2
On Tue, Jun 26, 2018 at 4:24 AM, Vik Fearing <[hidden email]> wrote:
Looks good to me.

I'll second that. Looks good to me too. 
Reply | Threaded
Open this post in threaded view
|

Re: Constraint documentation

Peter Eisentraut-6
In reply to this post by Lætitia Avrot
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

Reply | Threaded
Open this post in threaded view
|

Re: Constraint documentation

Fabien COELHO-3

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.

Reply | Threaded
Open this post in threaded view
|

Re: Constraint documentation

Peter Eisentraut-6
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

Reply | Threaded
Open this post in threaded view
|

Re: Constraint documentation

Fabien COELHO-3

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.
Reply | Threaded
Open this post in threaded view
|

Re: Constraint documentation

Lætitia Avrot
In reply to this post by Peter Eisentraut-6
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 <[hidden email]> 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.
Reply | Threaded
Open this post in threaded view
|

Re: Constraint documentation

Alvaro Herrera-9
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

Reply | Threaded
Open this post in threaded view
|

Re: Constraint documentation

Pantelis Theodosiou


On Thu, Aug 9, 2018 at 10:32 PM, Alvaro Herrera <[hidden email]> 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
Reply | Threaded
Open this post in threaded view
|

Re: Constraint documentation

Peter Eisentraut-6
In reply to this post by Alvaro Herrera-9
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

Reply | Threaded
Open this post in threaded view
|

Re: Constraint documentation

Tom Lane-2
Peter Eisentraut <[hidden email]> 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

Reply | Threaded
Open this post in threaded view
|

Re: Constraint documentation

David Fetter
In reply to this post by Peter Eisentraut-6
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

12
Previous Thread Next Thread