Thread: Check constraints on non-immutable keys
We currently allow this: postgres=# create table t(a timestamptz not null primary key, check(a > now())); NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "t_pkey" for table "t" CREATE TABLE Which seems very wrong. For one thing, a dump of this database can not be restored if now() has advanced enough into the future (which it will eventually). It also makes impossible to do things like SET a=a on the table. Yes, this is clearly a stupidly defined constraint, but why do we allow it? Shouldn't we disallow anything that's not IMMUTABLE in a check constraint? -- Magnus HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/
Magnus Hagander <magnus@hagander.net> writes: > Shouldn't we disallow anything that's not IMMUTABLE in a check constraint? I think you'd get too many howls of pain ... also, such a restriction is likely contrary to SQL spec. The example seems to me to be in the category of "so don't do that" rather than something that we need to save users from. Yes, it's stupid, but for example the very similar check of "a < now()" would be perfectly safe and arguably useful. regards, tom lane
On Wed, Jun 30, 2010 at 16:38, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Magnus Hagander <magnus@hagander.net> writes: >> Shouldn't we disallow anything that's not IMMUTABLE in a check constraint? > > I think you'd get too many howls of pain ... also, such a restriction is > likely contrary to SQL spec. Really? That sounds strange, but I can't claim to k now the SQL spec.. > The example seems to me to be in the category of "so don't do that" > rather than something that we need to save users from. Yes, it's In that case, should we at least throw a warning? > stupid, but for example the very similar check of "a < now()" > would be perfectly safe and arguably useful. Well, things like timezone settings could come in effect there as well... -- Magnus HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/
Magnus Hagander <magnus@hagander.net> writes: > On Wed, Jun 30, 2010 at 16:38, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> The example seems to me to be in the category of "so don't do that" >> rather than something that we need to save users from. �Yes, it's > In that case, should we at least throw a warning? I don't see a reason to do that. If we could distinguish actually problematic cases from safe cases, it would be helpful, but we can't. Moreover, throwing a warning would encourage people to do actively *unsafe* things to suppress the warning --- like marking functions as immutable when they really aren't. regards, tom lane
On Wed, Jun 30, 2010 at 11:02 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Magnus Hagander <magnus@hagander.net> writes: >> On Wed, Jun 30, 2010 at 16:38, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> The example seems to me to be in the category of "so don't do that" >>> rather than something that we need to save users from. Yes, it's > >> In that case, should we at least throw a warning? > > I don't see a reason to do that. If we could distinguish actually > problematic cases from safe cases, it would be helpful, but we can't. > > Moreover, throwing a warning would encourage people to do actively > *unsafe* things to suppress the warning --- like marking functions > as immutable when they really aren't. My scintillating contribution to this discussion is the observation that unrestorable dumps suck. A lot. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company
Robert Haas <robertmhaas@gmail.com> writes: > My scintillating contribution to this discussion is the observation > that unrestorable dumps suck. No doubt, but is this a real problem in practice? I can't recall many field complaints about it. And the ones I do recall wouldn't have been prevented by a check as stupid as "are there immutable functions in here". I still say that what such a check is likely to do is encourage people to mis-label mutable functions as immutable ... which will cause them a lot of *other* headaches. regards, tom lane
On Wed, Jun 30, 2010 at 11:49 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> My scintillating contribution to this discussion is the observation >> that unrestorable dumps suck. > > No doubt, but is this a real problem in practice? Magnus tells me that that was what prompted his original email. > I can't recall many > field complaints about it. And the ones I do recall wouldn't have been > prevented by a check as stupid as "are there immutable functions in > here". Hopefully there aren't too many ways to get data into a table that doesn't satisfy its check constraint - what else are you thinking of? Short of direct system catalog manipulation with malice aforethought, redefining a function to return different results after the fact is the only other case I can think of, and I'd propose we block that somehow too if I could figure out how. > I still say that what such a check is likely to do is encourage > people to mis-label mutable functions as immutable ... which will cause > them a lot of *other* headaches. If it does, those headaches are their fault, whereas this one, at least as I see it, is our fault. The fact that you can injure yourself badly with a sharp knife is not an excuse for someone to hand it to you pointy-end-first. I think it would be useful to have check constraints that are only enforced on new data, and allowing immutable functions there would make sense. But I can't think of any reasonable use case for having a non-immutable check constraint of the type we have now. Can you? Besides breaking pg_dump, it can also potentially foul up constraint exclusion. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company
On 30/06/10 17:11, Robert Haas wrote: > On Wed, Jun 30, 2010 at 11:49 AM, Tom Lane<tgl@sss.pgh.pa.us> wrote: >> Robert Haas<robertmhaas@gmail.com> writes: >>> My scintillating contribution to this discussion is the observation >>> that unrestorable dumps suck. >> >> No doubt, but is this a real problem in practice? > > Magnus tells me that that was what prompted his original email. I've done it. Luckily only with a small and fully functioning database so I could drop the constraint and re-dump it. Had a "recent_date" domain that was making sure new diary-style entries had a plausible date. Of course, two years later my dump can no longer restore the oldest record :-( IMHO The real solution would be something that could strip/rewrite the constraint on restore rather than trying to prevent people being stupid though. People *will* just tag their functions as immutable to get them to work. -- Richard Huxton Archonet Ltd
Robert Haas <robertmhaas@gmail.com> writes: > On Wed, Jun 30, 2010 at 11:49 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> I can't recall many >> field complaints about it. �And the ones I do recall wouldn't have been >> prevented by a check as stupid as "are there immutable functions in >> here". > Hopefully there aren't too many ways to get data into a table that > doesn't satisfy its check constraint - what else are you thinking of? Nobody is talking about having bypassed a check constraint --- the problem here is what if the "same" constraint condition is true today and false tomorrow. The cases that I can recall were not directly about time passing, but rather about check constraints that were designed to examine the contents of other tables or other rows in the same table. Functions that do that are properly declared STABLE not VOLATILE, but they'd still be rejected by Magnus' proposed restriction. The problem is that people would be *very* likely to just mark them IMMUTABLE rather than understand that what they're trying is fundamentally unreliable. That would cause them other problems, and they'd still be at risk of their dumps not reloading. I concur with the thought that the most useful solution might be a way to tell pg_restore to remove or disable check constraints. regards, tom lane
On 30/06/10 18:11, Magnus Hagander wrote: > On Wed, Jun 30, 2010 at 18:33, Richard Huxton<dev@archonet.com> wrote: >> >> IMHO The real solution would be something that could strip/rewrite the >> constraint on restore rather than trying to prevent people being stupid >> though. People *will* just tag their functions as immutable to get them to >> work. > > Are you sure? The people most likely to "just tag their functions as > immutable", are the same ones most unlikely to know *how to do that*. > At least for what I think is the majority case - which is calling > builtin functions. People just cut and paste this stuff from ancient blog entries. Understanding is not necessary. Hell, I do it sometimes if I'm dealing with something like LDAP where I don't really have a deep knowledge of the situation. -- Richard Huxton Archonet Ltd
On Wed, Jun 30, 2010 at 18:33, Richard Huxton <dev@archonet.com> wrote: > On 30/06/10 17:11, Robert Haas wrote: >> >> On Wed, Jun 30, 2010 at 11:49 AM, Tom Lane<tgl@sss.pgh.pa.us> wrote: >>> >>> Robert Haas<robertmhaas@gmail.com> writes: >>>> >>>> My scintillating contribution to this discussion is the observation >>>> that unrestorable dumps suck. >>> >>> No doubt, but is this a real problem in practice? >> >> Magnus tells me that that was what prompted his original email. > > I've done it. Luckily only with a small and fully functioning database so I > could drop the constraint and re-dump it. > > Had a "recent_date" domain that was making sure new diary-style entries had > a plausible date. Of course, two years later my dump can no longer restore > the oldest record :-( > > IMHO The real solution would be something that could strip/rewrite the > constraint on restore rather than trying to prevent people being stupid > though. People *will* just tag their functions as immutable to get them to > work. Are you sure? The people most likely to "just tag their functions as immutable", are the same ones most unlikely to know *how to do that*. At least for what I think is the majority case - which is calling builtin functions. -- Magnus HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/
On Wed, Jun 30, 2010 at 19:16, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> On Wed, Jun 30, 2010 at 11:49 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> I can't recall many >>> field complaints about it. And the ones I do recall wouldn't have been >>> prevented by a check as stupid as "are there immutable functions in >>> here". > >> Hopefully there aren't too many ways to get data into a table that >> doesn't satisfy its check constraint - what else are you thinking of? > > Nobody is talking about having bypassed a check constraint --- the > problem here is what if the "same" constraint condition is true today > and false tomorrow. The cases that I can recall were not directly about > time passing, but rather about check constraints that were designed to > examine the contents of other tables or other rows in the same table. > Functions that do that are properly declared STABLE not VOLATILE, but > they'd still be rejected by Magnus' proposed restriction. The problem > is that people would be *very* likely to just mark them IMMUTABLE rather > than understand that what they're trying is fundamentally unreliable. > That would cause them other problems, and they'd still be at risk of > their dumps not reloading. > > I concur with the thought that the most useful solution might be a way > to tell pg_restore to remove or disable check constraints. Uh, say what? Are you saying pg_restore should actually remove something from the database schema? And thus no longer be valid for taking database backups? Or are you just saying that it should have the constraints off, load the data, and then somehow create the constraint without having it validate the exinsting data (like the NOCHECK option in MSSQL?) -- Magnus HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/
On Wed, Jun 30, 2010 at 9:47 AM, Magnus Hagander <magnus@hagander.net> wrote: > We currently allow this: > > postgres=# create table t(a timestamptz not null primary key, check(a > now())); > NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index > "t_pkey" for table "t" > CREATE TABLE > > > Which seems very wrong. For one thing, a dump of this database can not > be restored if now() has advanced enough into the future (which it > will eventually). It also makes impossible to do things like SET a=a > on the table. > > Yes, this is clearly a stupidly defined constraint, but why do we allow it? > > Shouldn't we disallow anything that's not IMMUTABLE in a check constraint? suppose you did do this: shouldn't you then also recheck the constraint if the function is create/replaced? merlin
Magnus Hagander <magnus@hagander.net> writes: > On Wed, Jun 30, 2010 at 19:16, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> I concur with the thought that the most useful solution might be a way >> to tell pg_restore to remove or disable check constraints. > Uh, say what? Are you saying pg_restore should actually remove > something from the database schema? And thus no longer be valid for > taking database backups? pg_restore, not pg_dump. It's no more unreasonable an idea than the current pg_restore options for selective restores, AFAICS. You can already cause pg_restore to not restore PK and FK constraints, for example, so why not check constraints? regards, tom lane
On Wed, Jun 30, 2010 at 20:13, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Magnus Hagander <magnus@hagander.net> writes: >> On Wed, Jun 30, 2010 at 19:16, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> I concur with the thought that the most useful solution might be a way >>> to tell pg_restore to remove or disable check constraints. > >> Uh, say what? Are you saying pg_restore should actually remove >> something from the database schema? And thus no longer be valid for >> taking database backups? > > pg_restore, not pg_dump. It's no more unreasonable an idea than the > current pg_restore options for selective restores, AFAICS. You can > already cause pg_restore to not restore PK and FK constraints, for > example, so why not check constraints? Oh, sorry, I misread that - I thought you suggested it would do so by default. Clearly, I should've left work about 2 minutes earlier and not bothered you with that response :-) -- Magnus HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/
On ons, 2010-06-30 at 10:38 -0400, Tom Lane wrote: > Magnus Hagander <magnus@hagander.net> writes: > > Shouldn't we disallow anything that's not IMMUTABLE in a check constraint? > > I think you'd get too many howls of pain ... also, such a restriction is > likely contrary to SQL spec. <kibo> "The <search condition> shall simply contain a <boolean value expression> that is retrospectively deterministic." This is then defined in a rather complex manner that ends up disallowing col > now() but allowing col < now(). </kibo>
Peter Eisentraut <peter_e@gmx.net> writes: > <kibo> > "The <search condition> shall simply contain a <boolean value > expression> that is retrospectively deterministic." > This is then defined in a rather complex manner that ends up disallowing > col > now() but allowing col < now(). > </kibo> Oh, cute. Seems to have been added in SQL:2003. I guess somebody nagged them about wanting to be able to write CHECK(col <= now()). The detailed definition is amazingly laborious and yet limited, though, as it basically doesn't address the problem except for that specific case and close relatives. regards, tom lane
On Wed, Jun 30, 2010 at 6:57 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Peter Eisentraut <peter_e@gmx.net> writes: >> <kibo> >> "The <search condition> shall simply contain a <boolean value >> expression> that is retrospectively deterministic." > >> This is then defined in a rather complex manner that ends up disallowing >> col > now() but allowing col < now(). >> </kibo> > > Oh, cute. Seems to have been added in SQL:2003. I guess somebody > nagged them about wanting to be able to write CHECK(col <= now()). > The detailed definition is amazingly laborious and yet limited, though, > as it basically doesn't address the problem except for that specific > case and close relatives. Well, solving the problem in general is equivalent to the halting problem, so... -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company
magnus@hagander.net (Magnus Hagander) writes: >> I concur with the thought that the most useful solution might be a way >> to tell pg_restore to remove or disable check constraints. > > Uh, say what? Are you saying pg_restore should actually remove > something from the database schema? And thus no longer be valid for > taking database backups? > > Or are you just saying that it should have the constraints off, load > the data, and then somehow create the constraint without having it > validate the exinsting data (like the NOCHECK option in MSSQL?) Well, consider the way that foreign keys are handled by pg_dump. - Initially, it dumps out the table schema, NOT replete with foreign key constraints. - Data is loaded, *without* checking foreign keys. - Foreign keys are added in, afterwards. That's not a scenario where constraints are ignored - their evaluation is merely deferred. For constraints that involve dates, I can certainly see a potential for foot guns. It points me towards making sure that our apps don't do over-aggressive things like having constraints to prevent data from being inserted back-dated, as that would cause restores of backups to break. That's a dangerous kind of constraint. It's *possible* that it would be an idea to apply the check constraint late in the pg_dump, so that the ill effects might be imagined to be alleviated. That seems rather wishful. -- output = ("cbbrowne" "@" "gmail.com") http://linuxdatabases.info/info/languages.html HEADLINE: Suicidal twin kills sister by mistake!
Robert Haas <robertmhaas@gmail.com> writes: > On Wed, Jun 30, 2010 at 6:57 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> The detailed definition is amazingly laborious and yet limited, though, >> as it basically doesn't address the problem except for that specific >> case and close relatives. > Well, solving the problem in general is equivalent to the halting problem, so... So is proving determinism. They had the sense to *not* try to define what that means. regards, tom lane