Re: Check-out mutable functions in check constraints

On Fri, Jul 12, 2019 at 02:00:25PM +0200, Pavel Stehule wrote:
>pá 12. 7. 2019 v 13:11 odesílatel Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
>napsal:
>
>> On Fri, Jul 12, 2019 at 08:55:20AM +0200, Pavel Stehule wrote:
>> >Hi
>> >
>> >pá 12. 7. 2019 v 8:45 odesílatel Kyotaro Horiguchi <
>> horikyota(dot)ntt(at)gmail(dot)com>
>> >napsal:
>> >
>> >> Hello.
>> >>
>> >> As mentioned in the following message:
>> >>
>> >>
>> >>
>> https://www.postgresql.org/message-id/20190712.150527.145133646.horikyota.ntt%40gmail.com
>> >>
>> >> Mutable function are allowed in check constraint expressions but
>> >> it is not right. The attached is a proposed fix for it including
>> >> regression test.
>> >>
>> >> Other "constraints vs xxxx" checks do not seem to be exercised
>> >> but it would be another issue.
>> >>
>> >
>> >I think so this feature (although is correct) can breaks almost all
>> >applications - it is 20 year late.
>> >
>>
>> I'm not sure it actually breaks such appliations.
>>
>> Let's assume you have a mutable function (i.e. it may change return value
>> even with the same parameters) and you use it in a CHECK constraint. Then
>> I'm pretty sure your application is already broken in various ways and you
>> just don't know it (sometimes it subtle, sometimes less so).
>>
>
>Years ago SQL functions was used for checks instead triggers - I am not
>sure if this pattern was in documentation or not, but surely there was not
>any warning against it.
>
>You can see some documents with examples
>
>CREATE OR REPLACE FUNCTION check_func(int)
>RETURNS boolean AS $$
>SELECT 1 FROM tab WHERE id = $1;
>$$ LANGUAGE sql;
>
>CREATE TABLE foo( ... id CHECK(check_func(id)));
>

Considering this does not work (e.g. because in READ COMMITTED mode you
won't see the effects of uncommitted DELETE), I'd say this is a quite
nice example of the breakage I mentioned before.

You might add locking and make it somewhat safer, but there will still
be plenty of holes (e.g. because you won't see new but not yet
committed records). But it can cause issues e.g. with pg_dump [1].

So IMHO this is more an argument for adding the proposed check ...

>
>
>> If you have a function that actually is immutable and it's just not marked
>> accordingly, then that only requires a single DDL to fix that during
>> upgrade. I don't think that's a massive issue.
>>
>
>These functions are stable, and this patch try to prohibit it.
>

Yes, and the question is whether this is the right thing to do (I think
it probably is).

OTOH, even if we prohibit mutable functions in check constraints, people
can still create triggers doing those checks (and shoot themselves in
the foot that way).

[1] https://www.postgresql.org/message-id/6753.1452274727%40sss.pgh.pa.us

regards

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