Thread: Mutable foreign key constraints
I happened to notice that Postgres will let you do
regression=# create table foo (id timestamp primary key);
CREATE TABLE
regression=# create table bar (ts timestamptz references foo);
CREATE TABLE
This strikes me as a pretty bad idea, because whether a particular
timestamp is equal to a particular timestamptz depends on your
timezone setting. Thus the constraint could appear to be violated
after a timezone change.
I'm inclined to propose rejecting FK constraints if the comparison
operator is not immutable. Among the built-in opclasses, the only
instances of non-immutable btree equality operators are
regression=# select amopopr::regoperator from pg_amop join pg_operator o on o.oid = amopopr join pg_proc p on p.oid =
oprcodewhere amopmethod=403 and amopstrategy=3 and provolatile != 'i';
amopopr
---------------------------------------------------------
=(date,timestamp with time zone)
=(timestamp without time zone,timestamp with time zone)
=(timestamp with time zone,date)
=(timestamp with time zone,timestamp without time zone)
(4 rows)
A possible objection is that if anybody has such a setup and
hasn't noticed a problem because they never change their
timezone setting, they might not appreciate us breaking it.
So I certainly wouldn't propose back-patching this. But
maybe we should add it as a foot-gun defense going forward.
Thoughts?
regards, tom lane
On Thursday, September 12, 2024, Tom Lane <tgl@sss.pgh.pa.us> wrote:
A possible objection is that if anybody has such a setup and
hasn't noticed a problem because they never change their
timezone setting, they might not appreciate us breaking it.
So I certainly wouldn't propose back-patching this. But
maybe we should add it as a foot-gun defense going forward.
I’m disinclined to begin enforcing this. If they got a volatile data type in a key column and don’t attempt to index the key, which would fail on the volatile side, I’d be mighty surprised. I don’t really have much sympathy for anyone who got themselves into the described position but I don’t see this unsafe enough to force a potentially large table rewrite on those that managed to build a fragile but functioning model.
I suggest adding the commentary and queries used to check for just such a situation to the “don’t do this page” of the wiki and there just explain while allowed for backward compatibility it is definitely not a recommended setup.
David J.
"David G. Johnston" <david.g.johnston@gmail.com> writes:
> On Thursday, September 12, 2024, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> A possible objection is that if anybody has such a setup and
>> hasn't noticed a problem because they never change their
>> timezone setting, they might not appreciate us breaking it.
>> So I certainly wouldn't propose back-patching this. But
>> maybe we should add it as a foot-gun defense going forward.
> I’m disinclined to begin enforcing this. If they got a volatile data type
> in a key column and don’t attempt to index the key, which would fail on the
> volatile side, I’d be mighty surprised.
Um, neither type is "volatile" and each can be indexed just fine.
It's the cross-type comparison required by the FK that brings the
hazard.
> I suggest adding the commentary and queries used to check for just such a
> situation to the “don’t do this page” of the wiki and there just explain
> while allowed for backward compatibility it is definitely not a recommended
> setup.
Yeah, that's a possible approach.
regards, tom lane
On Thu, 2024-09-12 at 17:33 -0400, Tom Lane wrote:
> I happened to notice that Postgres will let you do
>
> regression=# create table foo (id timestamp primary key);
> CREATE TABLE
> regression=# create table bar (ts timestamptz references foo);
> CREATE TABLE
>
> This strikes me as a pretty bad idea, because whether a particular
> timestamp is equal to a particular timestamptz depends on your
> timezone setting. Thus the constraint could appear to be violated
> after a timezone change.
>
> I'm inclined to propose rejecting FK constraints if the comparison
> operator is not immutable.
I think that is the only sane thing to do. Consider
test=> SHOW timezone;
TimeZone
═══════════════
Europe/Vienna
(1 row)
test=> INSERT INTO foo VALUES ('2024-09-13 12:00:00');
INSERT 0 1
test=> INSERT INTO bar VALUES ('2024-09-13 12:00:00+02');
INSERT 0 1
test=> SELECT * FROM foo JOIN bar ON foo.id = bar.ts;
id │ ts
═════════════════════╪════════════════════════
2024-09-13 12:00:00 │ 2024-09-13 12:00:00+02
(1 row)
test=> SET timezone = 'Asia/Kolkata';
SET
test=> SELECT * FROM foo JOIN bar ON foo.id = bar.ts;
id │ ts
════╪════
(0 rows)
test=> INSERT INTO foo VALUES ('2024-09-14 12:00:00');
INSERT 0 1
test=> INSERT INTO bar VALUES ('2024-09-14 12:00:00+02');
ERROR: insert or update on table "bar" violates foreign key constraint "bar_ts_fkey"
DETAIL: Key (ts)=(2024-09-14 15:30:00+05:30) is not present in table "foo".
That's very broken and should not be allowed.
> A possible objection is that if anybody has such a setup and
> hasn't noticed a problem because they never change their
> timezone setting, they might not appreciate us breaking it.
I hope that there are few cases of that in the field, and I think it
is OK to break them. After all, it can be fixed with a simple
ALTER TABLE foo ALTER id TYPE timestamptz;
If the session time zone is UTC, that wouldn't even require a rewrite.
I agree that it cannot be backpatched.
Yours,
Laurenz Albe
On 9/13/24 4:41 AM, Laurenz Albe wrote: > That's very broken and should not be allowed. +1 >> A possible objection is that if anybody has such a setup and >> hasn't noticed a problem because they never change their >> timezone setting, they might not appreciate us breaking it. > > I hope that there are few cases of that in the field, and I think it > is OK to break them. After all, it can be fixed with a simple > > ALTER TABLE foo ALTER id TYPE timestamptz; > > If the session time zone is UTC, that wouldn't even require a rewrite. > > I agree that it cannot be backpatched. I unfortunately suspect there might be more cases than we think in the field due to many people not understanding the difference between timestamp and timestamptz but the good thing is that timestamp/timestamptz are rare in foreign keys, even in composite ones. Since this is quite broken and does not have any real world usefulness I think we should just go ahead and disallow it and have a few people complain. Andreas
On 9/13/24 15:05, Andreas Karlsson wrote: > On 9/13/24 4:41 AM, Laurenz Albe wrote: >> That's very broken and should not be allowed. > > +1 > >>> A possible objection is that if anybody has such a setup and >>> hasn't noticed a problem because they never change their >>> timezone setting, they might not appreciate us breaking it. >> >> I hope that there are few cases of that in the field, and I think it >> is OK to break them. After all, it can be fixed with a simple >> >> ALTER TABLE foo ALTER id TYPE timestamptz; >> >> If the session time zone is UTC, that wouldn't even require a rewrite. >> >> I agree that it cannot be backpatched. > > I unfortunately suspect there might be more cases than we think in the > field due to many people not understanding the difference between > timestamp and timestamptz but the good thing is that > timestamp/timestamptz are rare in foreign keys, even in composite ones. It will become a lot more common with WITHOUT OVERLAPS, so I think it is important to fix this at the same time or earlier as that feature. > Since this is quite broken and does not have any real world usefulness I > think we should just go ahead and disallow it and have a few people > complain. +1 -- Vik Fearing
On 2024-09-12 Th 5:33 PM, Tom Lane wrote: > I happened to notice that Postgres will let you do > > regression=# create table foo (id timestamp primary key); > CREATE TABLE > regression=# create table bar (ts timestamptz references foo); > CREATE TABLE > > This strikes me as a pretty bad idea, because whether a particular > timestamp is equal to a particular timestamptz depends on your > timezone setting. Thus the constraint could appear to be violated > after a timezone change. > > I'm inclined to propose rejecting FK constraints if the comparison > operator is not immutable. Among the built-in opclasses, the only > instances of non-immutable btree equality operators are > > regression=# select amopopr::regoperator from pg_amop join pg_operator o on o.oid = amopopr join pg_proc p on p.oid = oprcodewhere amopmethod=403 and amopstrategy=3 and provolatile != 'i'; > amopopr > --------------------------------------------------------- > =(date,timestamp with time zone) > =(timestamp without time zone,timestamp with time zone) > =(timestamp with time zone,date) > =(timestamp with time zone,timestamp without time zone) > (4 rows) > > A possible objection is that if anybody has such a setup and > hasn't noticed a problem because they never change their > timezone setting, they might not appreciate us breaking it. > So I certainly wouldn't propose back-patching this. But > maybe we should add it as a foot-gun defense going forward. > > Thoughts? > > Isn't there an upgrade hazard here? People won't thank us if they can't now upgrade their clusters. If we can get around that then +1. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Andrew Dunstan <andrew@dunslane.net> writes:
> On 2024-09-12 Th 5:33 PM, Tom Lane wrote:
>> I'm inclined to propose rejecting FK constraints if the comparison
>> operator is not immutable.
> Isn't there an upgrade hazard here? People won't thank us if they can't
> now upgrade their clusters. If we can get around that then +1.
Yeah, they would have to fix the bad DDL before upgrading. It'd
be polite of us to add a pg_upgrade precheck for such cases,
perhaps.
regards, tom lane