Обсуждение: check constraint problem during COPY while pg_upgrade-ing

I have noticed another problem during pg_upgrade on a 9.1
cluster with 9.4 as the target.

Consider this sort of table

    create table therapy (
        pk serial primary key,
        description text,
        is_ongoing boolean not null,
        ts_end timestamp with time zone
    );

Now, business rules say that a particular therapy is either
ongoing or not. The end of therapy can be known or not.
However, if the therapy is ongoing the ts_end must be either
NULL or "in the future" at row INSERT/UPDATE time.

Consider this check constraint

    CHECK (
        (is_ongoing is false)
            OR
        (
            ((is_ongoing is true) AND (ts_end is null))
                OR
            ((is_ongoing is true) AND (ts_end > now()))
        )
    )

(I know this can logically be reduced. I wrote it this way to
be explicit about the intent.)

This works fine, the application (GNUmed) ensures INSERTS and
UPDATES do the right thing with .is_ongoing and .ts_end.

Now the following sequence happens:

- insert row with .is_ongoing=true and .ts_end=tomorrow()
- wait a week
- dump
- restore

The restore will fail because the inserted row contains
.is_ongoing=true and .ts_end<now() ...

Of course, dump/restore can't be expected to know about my
business rules so I wonder what the _suggested_ approach to
this requirement is ?

(Technically one could use a BEFORE INSERT/UPDATE trigger to
 check .ts_end and .is_ongoing.)

Thanks for any input,
Karsten
--
GPG key ID E4071346 @ eu.pool.sks-keyservers.net
E167 67FD A291 2BEA 73BD  4537 78B9 A9F9 E407 1346

On 02/24/2016 02:12 PM, Karsten Hilbert wrote:
> I have noticed another problem during pg_upgrade on a 9.1
> cluster with 9.4 as the target.
>
> Consider this sort of table
>
>     create table therapy (
>         pk serial primary key,
>         description text,
>         is_ongoing boolean not null,
>         ts_end timestamp with time zone
>     );
>
> Now, business rules say that a particular therapy is either
> ongoing or not. The end of therapy can be known or not.
> However, if the therapy is ongoing the ts_end must be either
> NULL or "in the future" at row INSERT/UPDATE time.
>
> Consider this check constraint
>
>     CHECK (
>         (is_ongoing is false)
>             OR
>         (
>             ((is_ongoing is true) AND (ts_end is null))
>                 OR
>             ((is_ongoing is true) AND (ts_end > now()))
>         )
>     )
>
> (I know this can logically be reduced. I wrote it this way to
> be explicit about the intent.)
>
> This works fine, the application (GNUmed) ensures INSERTS and
> UPDATES do the right thing with .is_ongoing and .ts_end.
>
> Now the following sequence happens:
>
> - insert row with .is_ongoing=true and .ts_end=tomorrow()
> - wait a week
> - dump
> - restore

Seems to be you are caught in a logical bind even with out the dump/restore.

At some point past tomorrow(), absent a change in is_ongoing, you will
have a row where is_ongoing is 't' but ts_end says the therapy is over.
To my way of thinking this means having ts_end be NULL until the therapy
is completed or have a periodic job that marks is_ongoing = 'f' when
ts_end goes into the past and is_ongoing = 't'. Otherwise resort to the
trigger method you suggest below.

>
> The restore will fail because the inserted row contains
> .is_ongoing=true and .ts_end<now() ...
>
> Of course, dump/restore can't be expected to know about my
> business rules so I wonder what the _suggested_ approach to
> this requirement is ?
>
> (Technically one could use a BEFORE INSERT/UPDATE trigger to
>   check .ts_end and .is_ongoing.)
>
> Thanks for any input,
> Karsten
>


--
Adrian Klaver
adrian.klaver@aklaver.com

On Wed, Feb 24, 2016 at 04:12:22PM -0800, Adrian Klaver wrote:

> >This works fine, the application (GNUmed) ensures INSERTS and
> >UPDATES do the right thing with .is_ongoing and .ts_end.
> >
> >Now the following sequence happens:
> >
> >- insert row with .is_ongoing=true and .ts_end=tomorrow()
> >- wait a week
> >- dump
> >- restore
>
> Seems to be you are caught in a logical bind even with out the dump/restore.

Absolutely, it's just that the dump/restore made me realize
the folly of my ways.

> At some point past tomorrow(), absent a change in is_ongoing, you will have
> a row where is_ongoing is 't' but ts_end says the therapy is over. To my way
> of thinking this means having ts_end be NULL until the therapy is completed
> or have a periodic job that marks is_ongoing = 'f' when ts_end goes into the
> past and is_ongoing = 't'. Otherwise resort to the trigger method you
> suggest below.

Which expects to rely on the fact that triggers are only
activated after the data has been restored, be it by SQL
INSERTs or by COPY. However the COPY docs say (under Notes):

    COPY FROM will invoke any triggers and check constraints
    on the destination table. However, it will not invoke
    rules.

So there :-)

I'll have to install a BEFORE INSERT/UPDATE trigger which
forces .is_ongoing to FALSE when

    ((.ts_end is not NULL) and .ts_end < now())

That should do the right thing.

Thanks for the input. Helpful as usual.

Karsten
--
GPG key ID E4071346 @ eu.pool.sks-keyservers.net
E167 67FD A291 2BEA 73BD  4537 78B9 A9F9 E407 1346

On Wed, Feb 24, 2016 at 05:24:44PM -0700, David G. Johnston wrote:

> ​Then you must record the "INSERT/UPDATE time" into the table, as a
> constant, and refer to that value instead of having "now()" which happens
> to be correct at the time of the insert/update but is not correct at any
> future point.
>
> So:
>
> ts_last_change DEFAULT now()
> and
> CHECK (...
> (is_ongoing is true) AND (ts_end >
> ​ts_last_change
> )
> ​)​
>
> ​In this way a restoration will restore both the historical ts_last_change
> and the value of is_ongoing that matched.​

Thanks for this suggestion. In fact, my table data audit
system already provides for such a column, namely
.modified_when.

However, I still like to have the trigger to set .is_ongoing
to FALSE when .ts_end goes into the past (as of the time of
an UPDATE to the row), regardless of the fact that the
constraint between .ts_end and $TS_LAST_CHANGE (here:
.modified_when) is formally fulfilled. The reason being that
that does not really represent the business intent of the
constraint, which is

    WHENEVER
        therapy.is_ongoing is true,
    THEN (at the time of WHENEVER)
        therapy.ts_end must be unknown OR in
        the (real world) future

:-)

Karsten
--
GPG key ID E4071346 @ eu.pool.sks-keyservers.net
E167 67FD A291 2BEA 73BD  4537 78B9 A9F9 E407 1346

On 02/25/2016 01:58 AM, Karsten Hilbert wrote:
> On Wed, Feb 24, 2016 at 04:12:22PM -0800, Adrian Klaver wrote:
>
>>> This works fine, the application (GNUmed) ensures INSERTS and
>>> UPDATES do the right thing with .is_ongoing and .ts_end.
>>>
>>> Now the following sequence happens:
>>>
>>> - insert row with .is_ongoing=true and .ts_end=tomorrow()
>>> - wait a week
>>> - dump
>>> - restore
>>
>> Seems to be you are caught in a logical bind even with out the dump/restore.
>
> Absolutely, it's just that the dump/restore made me realize
> the folly of my ways.
>
>> At some point past tomorrow(), absent a change in is_ongoing, you will have
>> a row where is_ongoing is 't' but ts_end says the therapy is over. To my way
>> of thinking this means having ts_end be NULL until the therapy is completed
>> or have a periodic job that marks is_ongoing = 'f' when ts_end goes into the
>> past and is_ongoing = 't'. Otherwise resort to the trigger method you
>> suggest below.
>
> Which expects to rely on the fact that triggers are only
> activated after the data has been restored, be it by SQL
> INSERTs or by COPY. However the COPY docs say (under Notes):
>
>     COPY FROM will invoke any triggers and check constraints
>     on the destination table. However, it will not invoke
>     rules.
>
> So there :-)

Well not entirely true ;) -

http://www.postgresql.org/docs/9.4/interactive/app-pgrestore.html

--disable-triggers

     This option is relevant only when performing a data-only restore.
It instructs pg_restore to execute commands to temporarily disable
triggers on the target tables while the data is reloaded. Use this if
you have referential integrity checks or other triggers on the tables
that you do not want to invoke during data reload.

     Presently, the commands emitted for --disable-triggers must be done
as superuser. So you should also specify a superuser name with -S or,
preferably, run pg_restore as a PostgreSQL superuser.

>
> I'll have to install a BEFORE INSERT/UPDATE trigger which
> forces .is_ongoing to FALSE when
>
>     ((.ts_end is not NULL) and .ts_end < now())
>
> That should do the right thing.
>
> Thanks for the input. Helpful as usual.
>
> Karsten
>


--
Adrian Klaver
adrian.klaver@aklaver.com