Thread: ALTER TABLE ... ADD FOREIGN KEY ... NOT ENFORCED
The new SQL Standard (SQL:2011) contains this:
"Table constraints are either enforced or not enforced. Domain
constraints and assertions are always enforced.", 4.17.2
The SQL Standard allows you to turn the checking on and off for CHECK
constraints, UNIQUE constraints and FOREIGN KEYS.
Which of those make sense for us, if any? The ability to create FKs
without checking all the data has been frequently requested to me over
many years. OTOH, I can't really see any point in turning on/off all of
the other aspects mentioned by the SQL Standard, especially indexes.
It's lots of work and seems likely to end with poorer data quality. And
the obvious thing is if you don't want a CHECK constraint, just drop
it...
My proposal is that we add a short and simple clause NOT ENFORCED onto
the ADD constraint syntax. So we have
ALTER TABLE foo
ADD FOREIGN KEY .... NOT ENFORCED;
The "enforced" state is not persisted - once added the FK is checked
every time. So there is no additional column on pg_constraint.
The benefit here is that we implement a capability that allows skipping
very long running SQL statements when required, and doesn't require too
much code. It has been discussed before on hackers, but that was before
it was part of the SQL Standard. Oracle has had this for years and it is
popular feature. We can expect other RDBMS to implement this feature,
now it is part of the standard.
If you want more than my good-bits-only proposal, it really isn't going
to happen for 9.1, and seems pretty pointless anyway.
Very short hack to implement this attached for discussion. No tests, not
even a compile - just showing how quick a patch this can be.
Thoughts? Alternative syntax?
--
Simon Riggs http://www.2ndQuadrant.com/books/
PostgreSQL Development, 24x7 Support, Training and Services
Attachment
Simon Riggs <simon@2ndQuadrant.com> writes:
> The new SQL Standard (SQL:2011) contains this:
> "Table constraints are either enforced or not enforced. Domain
> constraints and assertions are always enforced.", 4.17.2
> The SQL Standard allows you to turn the checking on and off for CHECK
> constraints, UNIQUE constraints and FOREIGN KEYS.
Huh? It allows you to postpone the check until commit. That's far from
not enforcing it.
regards, tom lane
On Sun, 2010-12-12 at 17:57 -0500, Tom Lane wrote: > Simon Riggs <simon@2ndQuadrant.com> writes: > > The new SQL Standard (SQL:2011) contains this: > > "Table constraints are either enforced or not enforced. Domain > > constraints and assertions are always enforced.", 4.17.2 > > > The SQL Standard allows you to turn the checking on and off for CHECK > > constraints, UNIQUE constraints and FOREIGN KEYS. > > Huh? It allows you to postpone the check until commit. That's far from > not enforcing it. "When a <commit statement> is executed, all enforced constraints are effectively checked and, if any enforced constraint is not satisfied, then an exception condition is raised and the SQL-transaction is terminated by an implicit <rollback statement>." This clearly implies that un-enforced constraints are not checked at commit. -- Simon Riggs http://www.2ndQuadrant.com/books/PostgreSQL Development, 24x7 Support, Training and Services
Simon Riggs <simon@2ndQuadrant.com> writes:
> On Sun, 2010-12-12 at 17:57 -0500, Tom Lane wrote:
>> Huh? It allows you to postpone the check until commit. That's far from
>> not enforcing it.
> This clearly implies that un-enforced constraints are not checked at
> commit.
[ shrug... ] I can't argue with you about what may or may not be in an
unpublished draft of an unratified version of the standard, since I
don't have a copy. But allow me to harbor doubts that they really
intend to allow someone to force a constraint to be considered valid
without any verification. This proposal strikes me as something mysql
would do, not the standards committee. (In particular, can a constraint
go from not-enforced to enforced state without getting checked at that
time?)
Even if you're reading the draft correctly, and the wording makes it
into a released standard, the implementation you propose would break our
code. The incremental FK checks are designed on the assumption that the
constraint condition held before; they aren't likely to behave very
sanely if the data is bad. I'd want to see a whole lot more analysis of
the resulting behavior before even considering an idea like this.
regards, tom lane
On Sun, Dec 12, 2010 at 6:20 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Simon Riggs <simon@2ndQuadrant.com> writes: >> On Sun, 2010-12-12 at 17:57 -0500, Tom Lane wrote: >>> Huh? It allows you to postpone the check until commit. That's far from >>> not enforcing it. > >> This clearly implies that un-enforced constraints are not checked at >> commit. > > [ shrug... ] I can't argue with you about what may or may not be in an > unpublished draft of an unratified version of the standard, since I > don't have a copy. But allow me to harbor doubts that they really > intend to allow someone to force a constraint to be considered valid > without any verification. This proposal strikes me as something mysql > would do, not the standards committee. (In particular, can a constraint > go from not-enforced to enforced state without getting checked at that > time?) > > Even if you're reading the draft correctly, and the wording makes it > into a released standard, the implementation you propose would break our > code. The incremental FK checks are designed on the assumption that the > constraint condition held before; they aren't likely to behave very > sanely if the data is bad. I'd want to see a whole lot more analysis of > the resulting behavior before even considering an idea like this. Wow, you've managed to bash Simon, MySQL, and the SQL standards committee all in one email. I'm not going to argue that careful analysis isn't needed before doing something like this - and, in particular, if we ever get inner-join removal, which I'm still hoping to do at some point, a foreign key that isn't actually guaranteed to be valid might result in queries returning different answers depending on whether or not a join is removed. I guess we'd have to define that as the user's problem for alleging a foreign-key relationship that doesn't truly exist. On the other hand, there's clearly also a use case for this behavior. If a bulk load of prevalidated data forces an expensive revalidation of constraints that are already known to hold, there's a real chance the DBA will be backed into a corner where he simply has no choice but to not use foreign keys, even though he might really want to validate the foreign-key relationships on a going-forward basis. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> ... On the
> other hand, there's clearly also a use case for this behavior. If a
> bulk load of prevalidated data forces an expensive revalidation of
> constraints that are already known to hold, there's a real chance the
> DBA will be backed into a corner where he simply has no choice but to
> not use foreign keys, even though he might really want to validate the
> foreign-key relationships on a going-forward basis.
There may well be a case to be made for doing this on grounds of
practical usefulness. I'm just voicing extreme skepticism that it can
be supported by reference to the standard.
Personally I'd prefer to see us look into whether we couldn't arrange
for low-impact establishment of a verified FK relationship, analogous to
CREATE INDEX CONCURRENTLY. We don't let people just arbitrarily claim
that a uniqueness condition exists, and ISTM that if we can handle that
case we probably ought to be able to handle FK checking similarly.
regards, tom lane
On Sun, Dec 12, 2010 at 7:07 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> ... On the >> other hand, there's clearly also a use case for this behavior. If a >> bulk load of prevalidated data forces an expensive revalidation of >> constraints that are already known to hold, there's a real chance the >> DBA will be backed into a corner where he simply has no choice but to >> not use foreign keys, even though he might really want to validate the >> foreign-key relationships on a going-forward basis. > > There may well be a case to be made for doing this on grounds of > practical usefulness. I'm just voicing extreme skepticism that it can > be supported by reference to the standard. Dunno, I haven't read it either. But it does seem like the natural interpretation of "NOT ENFORCED". > Personally I'd prefer to see us look into whether we couldn't arrange > for low-impact establishment of a verified FK relationship, analogous to > CREATE INDEX CONCURRENTLY. We don't let people just arbitrarily claim > that a uniqueness condition exists, and ISTM that if we can handle that > case we probably ought to be able to handle FK checking similarly. That'd be useful, too, but I don't think it would remove the use case for skipping the check altogether. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Sun, Dec 12, 2010 at 4:49 PM, Robert Haas <robertmhaas@gmail.com> wrote: > On Sun, Dec 12, 2010 at 6:20 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> Simon Riggs <simon@2ndQuadrant.com> writes: >>> On Sun, 2010-12-12 at 17:57 -0500, Tom Lane wrote: >>>> Huh? It allows you to postpone the check until commit. That's far from >>>> not enforcing it. >> >>> This clearly implies that un-enforced constraints are not checked at >>> commit. >> >> [ shrug... ] I can't argue with you about what may or may not be in an >> unpublished draft of an unratified version of the standard, since I >> don't have a copy. But allow me to harbor doubts that they really >> intend to allow someone to force a constraint to be considered valid >> without any verification. This proposal strikes me as something mysql >> would do, not the standards committee. (In particular, can a constraint >> go from not-enforced to enforced state without getting checked at that >> time?) >> >> Even if you're reading the draft correctly, and the wording makes it >> into a released standard, the implementation you propose would break our >> code. The incremental FK checks are designed on the assumption that the >> constraint condition held before; they aren't likely to behave very >> sanely if the data is bad. I'd want to see a whole lot more analysis of >> the resulting behavior before even considering an idea like this. > > Wow, you've managed to bash Simon, MySQL, and the SQL standards > committee all in one email. > > I'm not going to argue that careful analysis isn't needed before doing > something like this - and, in particular, if we ever get inner-join > removal, which I'm still hoping to do at some point, a foreign key > that isn't actually guaranteed to be valid might result in queries > returning different answers depending on whether or not a join is > removed. I guess we'd have to define that as the user's problem for > alleging a foreign-key relationship that doesn't truly exist. On the > other hand, there's clearly also a use case for this behavior. If a > bulk load of prevalidated data forces an expensive revalidation of > constraints that are already known to hold, there's a real chance the > DBA will be backed into a corner where he simply has no choice but to > not use foreign keys, even though he might really want to validate the > foreign-key relationships on a going-forward basis. > > -- > Robert Haas > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company MySQL does in fact have this feature and it is used by mysqldump. This feature is very useful. -- Rob Wultsch wultsch@gmail.com
On 12/12/2010 08:27 PM, Rob Wultsch wrote: > > MySQL does in fact have this feature and it is used by mysqldump. This > feature is very useful. > The trouble is that FK's have more than one use. In particular, they have a documentary use that's used by tools that analyze databases, as well as by tools like htsql. They also have a role as an enforced constraint. In fact it's possible now to disable FK enforcement, by disabling the triggers. It's definitely a footgun though. Just the other day I was asked how data violating the constraint could have got into the table, and caused some surprise by demonstrating how easy this was to produce. So what would actually be an advance in my view would be a mechanism that allowed explicit disabling of a constraint but ensured that it was not violated when re-enabling it. cheers andrew
I wouldn't like to comment on whether or not Simon has correctly interpreted the words of the SQL standards committee, because standards committees sometimes word things in an intentionally ambiguous way to placate different interests, and because it seems fairly inconsequential in this case. IMHO this is a useful feature that should be pursued. There is another precedent that no one mentioned - DB2. From their docs: "You can add a foreign key with the NOT ENFORCED option to create an informational referential constraint. This action does not leave the table space in CHECK-pending status, and you do not need to execute CHECK DATA." I understand that DB2's informational referential constraints won't ever be enforced (they just show intent, which is useful to their planner), so this isn't really the same thing. However, DB2 apparently doesn't initially enforce referential integrity when an FK is created on a table with existing data, without any special syntax on the CREATE: "DB2 does not validate the data when you add the foreign key. Instead, if the table is populated....the table space that contains the table is placed in CHECK-pending status, just as if it had been loaded with ENFORCE NO. In this case, you need to execute the CHECK DATA utility to clear the CHECK-pending status." If I am not mistaken, this is almost exactly the behaviour described by Simon, because referential integrity is, presumably, enforced after the FK is created, but before the CHECK DATA utility is optionally run to ensure that we actually have referential integrity at a later time. I believe that Simon's proposal is essentially sound. I don't know why CHECK DATA operates at the tablespace granularity rather than the FK granularity - IANADB2U. If we followed this behaviour, we wouldn't "let people just arbitrarily claim" that a referential condition exists - rather, we'd let them assert that it /ought/ to exist, and that it will be maintained going forward, and give them the option of verifying that assertion at a later time, after which it actually exists. Unfortunately, this refinement of Simon's proposal would probably entail adding an additional column to pg_constraint. -- Regards, Peter Geoghegan
Peter Geoghegan wrote: > If we followed this behaviour, we wouldn't "let people just > arbitrarily claim" that a referential condition exists - rather, > we'd let them assert that it /ought/ to exist, and that it will be > maintained going forward, and give them the option of verifying > that assertion at a later time, after which it actually exists. What you outline would be quite valuable to our shop. Under the law, the "custodians of the data" are the elected clerks of circuit court, and under state law and rules of the state supreme court we can't "clean up" even the most glaring apparent data problems without the OK of the elected official or his or her designee. We have a very complex schema (although no more complex than necessary to model the reality of the data) with hundreds of foreign key relationships. For various reasons (conversions from old systems, etc.), these relationships don't hold on all tables in all county databases. It would be desirable to have foreign key definitions define the intended relationships anyway, and very useful for them to prevent further data degradation. For those situations where we get a business analyst to work with clerk of court staff to clean up orphaned rows, it would be very slick to be able to run some statement (like CHECK DATA) to see if the cleanup is complete and successful and to flag that the constraint is now enforced. So +1 on what Peter outlined as current DB2 features in this regard. -Kevin
On Sun, Dec 12, 2010 at 7:24 PM, Andrew Dunstan <andrew@dunslane.net> wrote: > In fact it's possible now to disable FK enforcement, by disabling the > triggers. It's definitely a footgun though. Just the other day I was asked > how data violating the constraint could have got into the table, and caused > some surprise by demonstrating how easy this was to produce. Ugh. I have read the entire pg manual and I did not recall that footgun. At least in MySQL disabling fk's is explicit. There is something to be said for being able to tell the database: "Hey, hold my beer and watch this, it might be stupid but it is what we are going to do". The database telling it's user that is a much larger issue (and yes, MySQL is generally worse). The user at least gets to talk to db through sql, the database only really gets to talk to the user through errors and the manual. The fact that fk checks are implemented by the trigger system somehow seems "surprising". -- Rob Wultsch wultsch@gmail.com
On Sun, 2010-12-12 at 19:07 -0500, Tom Lane wrote: > Robert Haas <robertmhaas@gmail.com> writes: > > ... On the > > other hand, there's clearly also a use case for this behavior. If a > > bulk load of prevalidated data forces an expensive revalidation of > > constraints that are already known to hold, there's a real chance the > > DBA will be backed into a corner where he simply has no choice but to > > not use foreign keys, even though he might really want to validate the > > foreign-key relationships on a going-forward basis. > > There may well be a case to be made for doing this on grounds of > practical usefulness. I'm just voicing extreme skepticism that it can > be supported by reference to the standard. > > Personally I'd prefer to see us look into whether we couldn't arrange > for low-impact establishment of a verified FK relationship, analogous to > CREATE INDEX CONCURRENTLY. We don't let people just arbitrarily claim > that a uniqueness condition exists, and ISTM that if we can handle that > case we probably ought to be able to handle FK checking similarly. I think we should do *both* things. Sometimes you already know the check will pass, sometimes you don't. In particular, reloading data from another source where you knew the checks passed. Enforcing re-checking in that case reduces data availability. -- Simon Riggs http://www.2ndQuadrant.com/books/PostgreSQL Development, 24x7 Support, Training and Services
Tom Lane <tgl@sss.pgh.pa.us> writes: > The incremental FK checks are designed on the assumption that the > constraint condition held before; they aren't likely to behave very > sanely if the data is bad. I'd want to see a whole lot more analysis of > the resulting behavior before even considering an idea like this. ALTER TABLE foo DISABLE TRIGGER ALL; I must have missed the point when PostgreSQL stoped providing this foot gun already, so that it's arguable less a surprise to spell the misfeature NOT ENFORCED rather than DISABLE TRIGGER. Seriously, real-world use cases such as Kevin's one seems to warrant that we are able to create a table withouth enforcing the FK. That's horrid, yes, that's needed, too. Maybe some operations would have to be instructed that the constraint ain't trustworthy but just declared to be so by the user? Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
2010/12/13 Tom Lane <tgl@sss.pgh.pa.us>: > But allow me to harbor doubts that they really > intend to allow someone to force a constraint to be considered valid > without any verification. "Table constraints are either enforced or not enforced. Domain constraints and assertions are always enforced.", 4.17.2 I don't read that as meaning that such unenforced constraints are considered "valid". It sounds as if unenforced constraints are the same as non-existing constraints (think: constraint "templates"), possibly as a means to "remember" that they should be re-enabled at some point. I.e., marking a constraint as unenforced and then as enforced again would be a shortcut for removing and re-adding the constraint, while having the advantage that one doesn't have to keep a list of constraint definitions that must be re-added. > (In particular, can a constraint > go from not-enforced to enforced state without getting checked at that > time?) I assume not. Nicolas
On 13 December 2010 10:30, Dimitri Fontaine <dimitri@2ndquadrant.fr> wrote: > Seriously, real-world use cases such as Kevin's one seems to warrant > that we are able to create a table withouth enforcing the FK. That's > horrid, yes, that's needed, too. Maybe some operations would have to be > instructed that the constraint ain't trustworthy but just declared to be > so by the user? Might I suggest that we call them "aspirational foreign keys", while sticking with Simon's syntax? Reasons: 1. It's suggestive of the lack of certainty about the referential integrity of the underlying data - They're like a foreign key, but not quite as good. 2. It's indicative that they may one day become actual foreign keys through the use of something like the CHECK DATA utility. I'd favour doing this with a separate DDL command. 3. It's suggestive that they aren't just syntactic sugar or an expression of intent, as DB2's NOT ENFORCED FKs are, but rather that they behave like Oracle's NOT ENFORCED FKs. 4. It's memorable, I think. By the way, the DISABLE TRIGGER ALL method isn't equivalent to this. Apart from hackishly depending on an implementation detail, it isn't possible to prevent the big, up-front enforcement in the first place when the FK is declared, because DISABLE TRIGGER ALL only disables existing triggers. Perhaps, if and when this feature is implemented, it will also be possible to use some explicit mechanism to disable and re-enable an FK. However, that's secondary I think. -- Regards, Peter Geoghegan
On Mon, Dec 13, 2010 at 12:59 AM, Rob Wultsch <wultsch@gmail.com> wrote: > On Sun, Dec 12, 2010 at 7:24 PM, Andrew Dunstan <andrew@dunslane.net> wrote: >> In fact it's possible now to disable FK enforcement, by disabling the >> triggers. It's definitely a footgun though. Just the other day I was asked >> how data violating the constraint could have got into the table, and caused >> some surprise by demonstrating how easy this was to produce. > > Ugh. I have read the entire pg manual and I did not recall that > footgun. At least in MySQL disabling fk's is explicit. There is > something to be said for being able to tell the database: "Hey, hold > my beer and watch this, it might be stupid but it is what we are going > to do". I couldn't agree more, and that's a great way to put it. The user is in charge. Our job is to prevent the user from *accidentally* shooting themselves in the foot. But if a crocodile is biting their foot off and they want to fire their gun in that direction and take their chances, it's not our job to say "oh, no, you might injure your foot". DBAs hate getting eaten by crocodiles. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Mon, 2010-12-13 at 11:54 +0000, Peter Geoghegan wrote:
> On 13 December 2010 10:30, Dimitri Fontaine <dimitri@2ndquadrant.fr> wrote:
> > Seriously, real-world use cases such as Kevin's one seems to warrant
> > that we are able to create a table withouth enforcing the FK. That's
> > horrid, yes, that's needed, too. Maybe some operations would have to be
> > instructed that the constraint ain't trustworthy but just declared to be
> > so by the user?
>
> Might I suggest that we call them "aspirational foreign keys", while
> sticking with Simon's syntax?
Just checking what we are saying:
1.
(a) ALTER TABLE ... ADD FOREIGN KEY ... NOT VALIDATED INITIALLY;
will add a FK but NOT run the check - we mark it as "check pending".
Lock held: ShareRowExclusiveLock
(b) Every new change to the table has the FK enforced - the triggers are
fully enabled and active. (That could mean we update a row and have the
update fail because of a FK violation.)
2. pg_validate_foreign_key('constraint name');
Returns immediately if FK is valid
Returns SETOF rows that violate the constraint, or if no rows are
returned it updates constraint to show it is now valid.
Lock held: AccessShareLock
Note that 1 & 2 together are the equivalent of ADD FK CONCURRENTLY,
except that step 2 more usefully tells you which rows fail.
-- Simon Riggs http://www.2ndQuadrant.com/books/PostgreSQL Development, 24x7 Support, Training and Services
On Mon, Dec 13, 2010 at 10:49 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
> (a) ALTER TABLE ... ADD FOREIGN KEY ... NOT VALIDATED INITIALLY;
> will add a FK but NOT run the check - we mark it as "check pending".
> Lock held: ShareRowExclusiveLock
Seems about right. Not sure whether the lock strength is correct.
> (b) Every new change to the table has the FK enforced - the triggers are
> fully enabled and active. (That could mean we update a row and have the
> update fail because of a FK violation.)
Also seems about right.
> 2. pg_validate_foreign_key('constraint name');
> Returns immediately if FK is valid
> Returns SETOF rows that violate the constraint, or if no rows are
> returned it updates constraint to show it is now valid.
> Lock held: AccessShareLock
I'm less sure about this part. I think there should be a DDL
statement to validate the foreign key. The "return the problem" rows
behavior could be done some other way, or just left to the user to
write their own query.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On 13 December 2010 16:08, Robert Haas <robertmhaas@gmail.com> wrote:
> On Mon, Dec 13, 2010 at 10:49 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
>> 2. pg_validate_foreign_key('constraint name');
>> Returns immediately if FK is valid
>> Returns SETOF rows that violate the constraint, or if no rows are
>> returned it updates constraint to show it is now valid.
>> Lock held: AccessShareLock
>
> I'm less sure about this part. I think there should be a DDL
> statement to validate the foreign key. The "return the problem" rows
> behavior could be done some other way, or just left to the user to
> write their own query.
+1. I think that a DDL statement is more appropriate, because it makes
the process sort of symmetrical.
Perhaps we could error on the first FK violation found, and give a
"value 'foo' not present in table bar". It ought to not matter that
there could be a lot of violations, because they will be exceptional
if you're using the feature as intended - presumably, you're going to
want to comb through the data to find out exactly what has gone wrong
for each violation. On the off chance that it actually is a problem,
the user can go ahead and write their own query, like Robert
suggested.
--
Regards,
Peter Geoghegan
On Mon, Dec 13, 2010 at 05:15:29PM +0000, Peter Geoghegan wrote:
> On 13 December 2010 16:08, Robert Haas <robertmhaas@gmail.com> wrote:
> > On Mon, Dec 13, 2010 at 10:49 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
> >> 2. pg_validate_foreign_key('constraint name');
> >> Returns immediately if FK is valid
> >> Returns SETOF rows that violate the constraint, or if no rows are
> >> returned it updates constraint to show it is now valid.
> >> Lock held: AccessShareLock
> >
> > I'm less sure about this part. I think there should be a DDL
> > statement to validate the foreign key. The "return the problem" rows
> > behavior could be done some other way, or just left to the user to
> > write their own query.
>
> +1. I think that a DDL statement is more appropriate, because it makes
> the process sort of symmetrical.
>
> Perhaps we could error on the first FK violation found, and give a
> "value 'foo' not present in table bar". It ought to not matter that
> there could be a lot of violations, because they will be exceptional
> if you're using the feature as intended - presumably, you're going to
> want to comb through the data to find out exactly what has gone wrong
> for each violation. On the off chance that it actually is a problem,
> the user can go ahead and write their own query, like Robert
> suggested.
Perhaps the errhint could suggest a query. All the information needed
for it would be available :)
Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
On Mon, 2010-12-13 at 17:15 +0000, Peter Geoghegan wrote:
> On 13 December 2010 16:08, Robert Haas <robertmhaas@gmail.com> wrote:
> > On Mon, Dec 13, 2010 at 10:49 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
> >> 2. pg_validate_foreign_key('constraint name');
> >> Returns immediately if FK is valid
> >> Returns SETOF rows that violate the constraint, or if no rows are
> >> returned it updates constraint to show it is now valid.
> >> Lock held: AccessShareLock
> >
> > I'm less sure about this part. I think there should be a DDL
> > statement to validate the foreign key. The "return the problem" rows
> > behavior could be done some other way, or just left to the user to
> > write their own query.
>
> +1. I think that a DDL statement is more appropriate, because it makes
> the process sort of symmetrical.
OK, sold.
> Perhaps we could error on the first FK violation found, and give a
> "value 'foo' not present in table bar". It ought to not matter that
> there could be a lot of violations, because they will be exceptional
> if you're using the feature as intended - presumably, you're going to
> want to comb through the data to find out exactly what has gone wrong
> for each violation. On the off chance that it actually is a problem,
> the user can go ahead and write their own query, like Robert
> suggested.
I think a function would help here, so I'll do that also.
-- Simon Riggs http://www.2ndQuadrant.com/books/PostgreSQL Development, 24x7 Support, Training and Services
tgl@sss.pgh.pa.us (Tom Lane) writes:
> Robert Haas <robertmhaas@gmail.com> writes:
>> ... On the
>> other hand, there's clearly also a use case for this behavior. If a
>> bulk load of prevalidated data forces an expensive revalidation of
>> constraints that are already known to hold, there's a real chance the
>> DBA will be backed into a corner where he simply has no choice but to
>> not use foreign keys, even though he might really want to validate the
>> foreign-key relationships on a going-forward basis.
>
> There may well be a case to be made for doing this on grounds of
> practical usefulness. I'm just voicing extreme skepticism that it can
> be supported by reference to the standard.
>
> Personally I'd prefer to see us look into whether we couldn't arrange
> for low-impact establishment of a verified FK relationship, analogous to
> CREATE INDEX CONCURRENTLY. We don't let people just arbitrarily claim
> that a uniqueness condition exists, and ISTM that if we can handle that
> case we probably ought to be able to handle FK checking similarly.
I can point to a use case that has proven useful...
Slony-I deactivates indices during the subscription process, because it
is enormously more efficient to load the data into the tables
sans-indices, and then re-index afterwards.
The same would apply for FK constraints.
I observe that the deactivation of indices is the sole remaining feature
in Slony-I that still requires catalog access in a "corruptive" sense.
(With the caveat that this corruption is now only a temporary one; the
indexes are returned into play before the subscription process
finishes.)
That would be eliminated by adding in: "ALTER TABLE ... DISABLE INDEX ..." "ALTER TABLE ... ENABLE INDEX ..."
For similar to apply to FK constraints would involve similar logic.
--
output = reverse("moc.liamg" "@" "enworbbc")
http://linuxdatabases.info/info/rdbms.html
"The code should be beaten into submission" -- Arthur Norman
Robert Haas wrote: > On Mon, Dec 13, 2010 at 12:59 AM, Rob Wultsch <wultsch@gmail.com> wrote: > > On Sun, Dec 12, 2010 at 7:24 PM, Andrew Dunstan <andrew@dunslane.net> wrote: > >> In fact it's possible now to disable FK enforcement, by disabling the > >> triggers. It's definitely a footgun though. Just the other day I was asked > >> how data violating the constraint could have got into the table, and caused > >> some surprise by demonstrating how easy this was to produce. > > > > Ugh. I have read the entire pg manual and I did not recall that > > footgun. ?At least in MySQL disabling fk's is explicit. There is > > something to be said for being able to tell the database: "Hey, hold > > my beer and watch this, it might be stupid but it is what we are going > > to do". > > I couldn't agree more, and that's a great way to put it. The user is > in charge. Our job is to prevent the user from *accidentally* > shooting themselves in the foot. But if a crocodile is biting their > foot off and they want to fire their gun in that direction and take > their chances, it's not our job to say "oh, no, you might injure your > foot". DBAs hate getting eaten by crocodiles. Is this a TODO? -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
On Wed, 2011-01-05 at 22:05 -0500, Bruce Momjian wrote: > Robert Haas wrote: > > On Mon, Dec 13, 2010 at 12:59 AM, Rob Wultsch <wultsch@gmail.com> wrote: > > > On Sun, Dec 12, 2010 at 7:24 PM, Andrew Dunstan <andrew@dunslane.net> wrote: > > >> In fact it's possible now to disable FK enforcement, by disabling the > > >> triggers. It's definitely a footgun though. Just the other day I was asked > > >> how data violating the constraint could have got into the table, and caused > > >> some surprise by demonstrating how easy this was to produce. > > > > > > Ugh. I have read the entire pg manual and I did not recall that > > > footgun. ?At least in MySQL disabling fk's is explicit. There is > > > something to be said for being able to tell the database: "Hey, hold > > > my beer and watch this, it might be stupid but it is what we are going > > > to do". > > > > I couldn't agree more, and that's a great way to put it. The user is > > in charge. Our job is to prevent the user from *accidentally* > > shooting themselves in the foot. But if a crocodile is biting their > > foot off and they want to fire their gun in that direction and take > > their chances, it's not our job to say "oh, no, you might injure your > > foot". DBAs hate getting eaten by crocodiles. > > Is this a TODO? The patch I'll be submitting, or getting eaten by crocodiles? -- Simon Riggs http://www.2ndQuadrant.com/books/PostgreSQL Development, 24x7 Support, Training and Services
On Mon, 2010-12-13 at 17:15 +0000, Peter Geoghegan wrote:
> On 13 December 2010 16:08, Robert Haas <robertmhaas@gmail.com> wrote:
> > On Mon, Dec 13, 2010 at 10:49 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
> >> 2. pg_validate_foreign_key('constraint name');
> >> Returns immediately if FK is valid
> >> Returns SETOF rows that violate the constraint, or if no rows are
> >> returned it updates constraint to show it is now valid.
> >> Lock held: AccessShareLock
> >
> > I'm less sure about this part. I think there should be a DDL
> > statement to validate the foreign key. The "return the problem" rows
> > behavior could be done some other way, or just left to the user to
> > write their own query.
>
> +1. I think that a DDL statement is more appropriate, because it makes
> the process sort of symmetrical.
Patch to implement the proposed feature attached, for CFJan2011.
2 sub-command changes:
ALTER TABLE foo ADD FOREIGN KEY fkoo ... NOT VALID;
ALTER TABLE foo VALIDATE CONSTRAINT fkoo;
--
Simon Riggs http://www.2ndQuadrant.com/books/
PostgreSQL Development, 24x7 Support, Training and Services
Attachment
On Jan 14, 2011, at 5:15 AM, Simon Riggs wrote:
> On Mon, 2010-12-13 at 17:15 +0000, Peter Geoghegan wrote:
>> On 13 December 2010 16:08, Robert Haas <robertmhaas@gmail.com> wrote:
>>> On Mon, Dec 13, 2010 at 10:49 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
>>>> 2. pg_validate_foreign_key('constraint name');
>>>> Returns immediately if FK is valid
>>>> Returns SETOF rows that violate the constraint, or if no rows are
>>>> returned it updates constraint to show it is now valid.
>>>> Lock held: AccessShareLock
>>>
>>> I'm less sure about this part. I think there should be a DDL
>>> statement to validate the foreign key. The "return the problem" rows
>>> behavior could be done some other way, or just left to the user to
>>> write their own query.
>>
>> +1. I think that a DDL statement is more appropriate, because it makes
>> the process sort of symmetrical.
>
> Patch to implement the proposed feature attached, for CFJan2011.
>
> 2 sub-command changes:
>
> ALTER TABLE foo ADD FOREIGN KEY fkoo ... NOT VALID;
>
> ALTER TABLE foo VALIDATE CONSTRAINT fkoo;
Sorry for the late reply; I just saw this...
Is there any way to be able to get the bad records out of the ALTER ... VALIDATE? I know it's pretty unusual, but for a
setof large tables, it could take hours to run a separate query that gives you the results.
BTW, I agree that this should be a DDL command, it would be very odd if it wasn't. But I also see it being very useful
tobe able to get the set of bad rows at the same time. Maybe if there was an SRF that did the real work and the ALTER
justignored the resultset?
--
Jim C. Nasby, Database Architect jim@nasby.net
512.569.9461 (cell) http://jim.nasby.net
On Tue, 2011-01-18 at 14:26 -0600, Jim Nasby wrote: > > > > 2 sub-command changes: > > > > ALTER TABLE foo ADD FOREIGN KEY fkoo ... NOT VALID; > > > > ALTER TABLE foo VALIDATE CONSTRAINT fkoo; > > Sorry for the late reply; I just saw this... > > Is there any way to be able to get the bad records out of the ALTER ... VALIDATE? I know it's pretty unusual, but for aset of large tables, it could take hours to run a separate query that gives you the results. > > BTW, I agree that this should be a DDL command, it would be very odd if it wasn't. But I also see it being very usefulto be able to get the set of bad rows at the same time. Maybe if there was an SRF that did the real work and the ALTERjust ignored the resultset? I agree. Unfortunately that wasn't the consensus when I proposed that earlier, so its just a simple validation true/false. I could add an SRF that ran the validation query but brought back the rows, but if zero then it sets valid. We could have both... -- Simon Riggs http://www.2ndQuadrant.com/books/PostgreSQL Development, 24x7 Support, Training and Services
Hi Simon, On 1/14/2011 1:15 PM, Simon Riggs wrote: > Patch to implement the proposed feature attached, for CFJan2011. Overall, I think the patch looks good, but I found some problems with it. In tablecmds.c you have: + if (found && con->contype == CONSTR_FOREIGN && !con->convalidated) which I don't think is correct, and my tests seem to agree; the actual validation doesn't happen at all. Changing that to CONSTRAINT_FOREIGN makes the validation part work, but then I get: ERROR: cache lookup failed for constraint 16419 when trying to drop the table and the regression tests fail because of this. Also having a regression test where the validation fails seems like a good idea. Another problem I found is that psql doesn't indicate in any way that a FOREIGN KEY constraint is not validated yet. I also think that having the function for getting a list of values that violate the constraint would be helpful. Any particular reason why you decided to omit it from this patch? Regards, Marko Tiikkaja
On Sun, 2011-01-23 at 19:50 +0200, Marko Tiikkaja wrote: > Hi Simon, > > On 1/14/2011 1:15 PM, Simon Riggs wrote: > > Patch to implement the proposed feature attached, for CFJan2011. > > Overall, I think the patch looks good Thanks for the review. > , but I found some problems with > it. In tablecmds.c you have: > > + if (found && con->contype == CONSTR_FOREIGN && !con->convalidated) > > which I don't think is correct, and my tests seem to agree; the actual > validation doesn't happen at all. Changing that to CONSTRAINT_FOREIGN > makes the validation part work, but then I get: > > ERROR: cache lookup failed for constraint 16419 > > when trying to drop the table and the regression tests fail because of > this. Also having a regression test where the validation fails seems > like a good idea. Thanks. Will fix. > Another problem I found is that psql doesn't indicate in any way that a > FOREIGN KEY constraint is not validated yet. Should it? What command do you think needs changing? > I also think that having the function for getting a list of values that > violate the constraint would be helpful. Any particular reason why you > decided to omit it from this patch? Yes, the consensus was that DDL was required, not a function. Function was my preferred approach originally. That now appears to be an additional request from a couple of people. At present, its easy enough to write the SQL statement yourself, so that's non-essential, and maybe/likely won't make this release (not sure, depends upon how other aspects go). There is no option to invoke this yet from pg_restore, which seems likely to top the list of priorities. Would you agree? -- Simon Riggs http://www.2ndQuadrant.com/books/PostgreSQL Development, 24x7 Support, Training and Services
On 1/23/2011 8:23 PM, Simon Riggs wrote: > On Sun, 2011-01-23 at 19:50 +0200, Marko Tiikkaja wrote: >> Another problem I found is that psql doesn't indicate in any way that a >> FOREIGN KEY constraint is not validated yet. > > Should it? > What command do you think needs changing? \d table now only shows that there's a FOREIGN KEY, which might lead the user to think that there should not be any values that don't exist in the referenced table. >> I also think that having the function for getting a list of values that >> violate the constraint would be helpful. Any particular reason why you >> decided to omit it from this patch? > > Yes, the consensus was that DDL was required, not a function. Function > was my preferred approach originally. While I do agree that the DDL command should be the preferred way to validate the constraint, I think the function adds a significant value when the validation does not succeed. > That now appears to be an additional request from a couple of people. At > present, its easy enough to write the SQL statement yourself, so that's > non-essential, and maybe/likely won't make this release (not sure, > depends upon how other aspects go). I understand. > There is no option to invoke this yet from pg_restore, which seems > likely to top the list of priorities. Would you agree? I don't understand what you mean with this. Could you be a bit more elaborate? Regards, Marko Tiikkaja
On Sun, 2011-01-23 at 20:33 +0200, Marko Tiikkaja wrote: > On 1/23/2011 8:23 PM, Simon Riggs wrote: > > On Sun, 2011-01-23 at 19:50 +0200, Marko Tiikkaja wrote: > >> Another problem I found is that psql doesn't indicate in any way that a > >> FOREIGN KEY constraint is not validated yet. > > > > Should it? > > What command do you think needs changing? > > \d table now only shows that there's a FOREIGN KEY, which might lead the > user to think that there should not be any values that don't exist in > the referenced table. Neither \d nor \di shows invalid indexes. Should we add validation for FKs when it is not there for indexes? My feeling was no. Desirable for both? Yes, but not as part of this patch. > > There is no option to invoke this yet from pg_restore, which seems > > likely to top the list of priorities. Would you agree? > > I don't understand what you mean with this. Could you be a bit more > elaborate? The purpose of this patch is performance. pg_restore will be faster if it uses this new feature, so I expect to add an option to reload data without validating FKs. -- Simon Riggs http://www.2ndQuadrant.com/books/PostgreSQL Development, 24x7 Support, Training and Services
On 1/23/2011 8:43 PM, Simon Riggs wrote: > On Sun, 2011-01-23 at 20:33 +0200, Marko Tiikkaja wrote: >> On 1/23/2011 8:23 PM, Simon Riggs wrote: >>> On Sun, 2011-01-23 at 19:50 +0200, Marko Tiikkaja wrote: >>>> Another problem I found is that psql doesn't indicate in any way that a >>>> FOREIGN KEY constraint is not validated yet. >>> >>> Should it? >>> What command do you think needs changing? >> >> \d table now only shows that there's a FOREIGN KEY, which might lead the >> user to think that there should not be any values that don't exist in >> the referenced table. > > Neither \d nor \di shows invalid indexes. What exactly are you referring to? An index with indisvalid=false looks like this in my psql: "fooindex" btree (a) INVALID And even if it didn't, I don't think we should be adding more deficiencies to psql. > Should we add validation for FKs when it is not there for indexes? > My feeling was no. > > Desirable for both? Yes, but not as part of this patch. > >>> There is no option to invoke this yet from pg_restore, which seems >>> likely to top the list of priorities. Would you agree? >> >> I don't understand what you mean with this. Could you be a bit more >> elaborate? > > The purpose of this patch is performance. pg_restore will be faster if > it uses this new feature, so I expect to add an option to reload data > without validating FKs. Ah. Right, that would make sense. Regards, Marko Tiikkaja
On Sun, 2011-01-23 at 20:56 +0200, Marko Tiikkaja wrote: > On 1/23/2011 8:43 PM, Simon Riggs wrote: > > On Sun, 2011-01-23 at 20:33 +0200, Marko Tiikkaja wrote: > >> On 1/23/2011 8:23 PM, Simon Riggs wrote: > >>> On Sun, 2011-01-23 at 19:50 +0200, Marko Tiikkaja wrote: > >>>> Another problem I found is that psql doesn't indicate in any way that a > >>>> FOREIGN KEY constraint is not validated yet. > >>> > >>> Should it? > >>> What command do you think needs changing? > >> > >> \d table now only shows that there's a FOREIGN KEY, which might lead the > >> user to think that there should not be any values that don't exist in > >> the referenced table. > > > > Neither \d nor \di shows invalid indexes. > > What exactly are you referring to? An index with indisvalid=false looks > like this in my psql: > > "fooindex" btree (a) INVALID > > And even if it didn't, I don't think we should be adding more > deficiencies to psql. OK, thanks, I wasn't aware of that. I'll add something similar for FKs. -- Simon Riggs http://www.2ndQuadrant.com/books/PostgreSQL Development, 24x7 Support, Training and Services
Simon Riggs <simon@2ndQuadrant.com> writes:
> On Sun, 2011-01-23 at 20:33 +0200, Marko Tiikkaja wrote:
>> \d table now only shows that there's a FOREIGN KEY, which might lead the
>> user to think that there should not be any values that don't exist in
>> the referenced table.
> Neither \d nor \di shows invalid indexes.
Even if that were true, it's a poor analogy, since a disabled foreign
key has visible *semantic* impact, whereas a disabled index doesn't.
regards, tom lane
On Sun, 2011-01-23 at 14:45 -0500, Tom Lane wrote: > Simon Riggs <simon@2ndQuadrant.com> writes: > > On Sun, 2011-01-23 at 20:33 +0200, Marko Tiikkaja wrote: > >> \d table now only shows that there's a FOREIGN KEY, which might lead the > >> user to think that there should not be any values that don't exist in > >> the referenced table. > > > Neither \d nor \di shows invalid indexes. > > Even if that were true, it's a poor analogy, since a disabled foreign > key has visible *semantic* impact, whereas a disabled index doesn't. Sure. My agreement to add something appears to have crossed with your comments. I'd appreciate you reviewing the parser aspects of the patch. $TITLE no longer reflects the syntax. -- Simon Riggs http://www.2ndQuadrant.com/books/PostgreSQL Development, 24x7 Support, Training and Services
On Tue, 2010-12-14 at 11:24 -0500, Chris Browne wrote: > tgl@sss.pgh.pa.us (Tom Lane) writes: > > Robert Haas <robertmhaas@gmail.com> writes: > >> ... On the > >> other hand, there's clearly also a use case for this behavior. If a > >> bulk load of prevalidated data forces an expensive revalidation of > >> constraints that are already known to hold, there's a real chance the > >> DBA will be backed into a corner where he simply has no choice but to > >> not use foreign keys, even though he might really want to validate the > >> foreign-key relationships on a going-forward basis. > > > > There may well be a case to be made for doing this on grounds of > > practical usefulness. I'm just voicing extreme skepticism that it can > > be supported by reference to the standard. > > > > Personally I'd prefer to see us look into whether we couldn't arrange > > for low-impact establishment of a verified FK relationship, analogous to > > CREATE INDEX CONCURRENTLY. We don't let people just arbitrarily claim > > that a uniqueness condition exists, and ISTM that if we can handle that > > case we probably ought to be able to handle FK checking similarly. > > I can point to a use case that has proven useful... > > Slony-I deactivates indices during the subscription process, because it > is enormously more efficient to load the data into the tables > sans-indices, and then re-index afterwards. > > The same would apply for FK constraints. > > I observe that the deactivation of indices is the sole remaining feature > in Slony-I that still requires catalog access in a "corruptive" sense. > (With the caveat that this corruption is now only a temporary one; the > indexes are returned into play before the subscription process > finishes.) > > That would be eliminated by adding in: > "ALTER TABLE ... DISABLE INDEX ..." > "ALTER TABLE ... ENABLE INDEX ..." > > For similar to apply to FK constraints would involve similar logic. I just wanted to point out that the patch submitted here does not allow what is requested here for FKs (nor indexes). You can add an FK without an initial check, but the FK is enforced for all further DML changes. You can then later validate the FK. So that running these commands ALTER TABLE foo ADD FOREIGN KEY ... NOT VALID; ALTER TABLE foo VALIDATE CONSTRAINT ...; is roughly equivalent to the concept of ALTER TABLE foo ADD FOREIGN KEY ... CONCURRENTLY; There is no command that makes the FK "NOT ENFORCED", so you can't turn it off then back on again as was requested above. -- Simon Riggs http://www.2ndQuadrant.com/books/PostgreSQL Development, 24x7 Support, Training and Services
Simon Riggs <simon@2ndQuadrant.com> writes:
> You can add an FK without an initial check, but the FK is enforced for
> all further DML changes.
I seem to recall pointing out upthread that the FK check triggers are
designed on the assumption that the constraint does hold currently.
Has any analysis been done on exactly how badly they'll fail when it
doesn't hold? I remain unconvinced that this behavior is desirable.
regards, tom lane
On Sun, 2011-01-23 at 16:13 -0500, Tom Lane wrote: > Simon Riggs <simon@2ndQuadrant.com> writes: > > You can add an FK without an initial check, but the FK is enforced for > > all further DML changes. > > I seem to recall pointing out upthread that the FK check triggers are > designed on the assumption that the constraint does hold currently. > Has any analysis been done on exactly how badly they'll fail when it > doesn't hold? I remain unconvinced that this behavior is desirable. If you mean RESTRICT relationships, then yes. I haven't foreseen other problems myself. What other ideas or risks would you like me to confirm or deny for you? -- Simon Riggs http://www.2ndQuadrant.com/books/PostgreSQL Development, 24x7 Support, Training and Services
Tom Lane wrote: > I seem to recall pointing out upthread that the FK check triggers > are designed on the assumption that the constraint does hold > currently. Has any analysis been done on exactly how badly they'll > fail when it doesn't hold? I remain unconvinced that this behavior > is desirable. I saw your upstream comment, and it took a quick look at it. On the face of it, I couldn't see where checking that the parent exists on the insert of a child would be broken by the existence of other orphan children, nor could I see where checking for the absence of children on the delete of a parent would be broken by orphan children not related to the parent. With other things on my plate I didn't have time to do a rigorous check, but it was enough to make me wonder what you think depends on existing consistency or what could break. Even a vague hint of what sort of thing you think might go wrong might help people find problem code, if it actually exists. Now, I understand that a broken index, like one based on a function declared immutable which really isn't, could cause problems, but that seems orthogonal. -Kevin
simon@2ndQuadrant.com (Simon Riggs) writes: > I just wanted to point out that the patch submitted here does not allow > what is requested here for FKs (nor indexes). That's fine; I was trying to support the thought that there was something useful about this idea. Being able to expressly deactivate indices seems like a reasonable thing to add as a separate TODO item, and I'll see about doing so. -- MICROS~1 has brought the microcomputer OS to the point where it is more bloated than even OSes from what was previously larger classes of machines altogether. This is perhaps Bill's single greatest accomplishment.
On Fri, Jan 14, 2011 at 6:15 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
> Patch to implement the proposed feature attached, for CFJan2011.
>
> 2 sub-command changes:
>
> ALTER TABLE foo ADD FOREIGN KEY fkoo ... NOT VALID;
>
> ALTER TABLE foo VALIDATE CONSTRAINT fkoo;
This patch, which seems to be the latest version, no longer applies,
and has not been updated based on the previous provided review
comments.
Also, this diff hunk looks scary to me:
+ if (must_use_query)
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("cannot SELECT from primary
key of relation \"%s\"",
+ RelationGetRelationName(rel))));
+
What's the justification for that?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Thu, Feb 3, 2011 at 11:00 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Fri, Jan 14, 2011 at 6:15 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
>> Patch to implement the proposed feature attached, for CFJan2011.
>>
>> 2 sub-command changes:
>>
>> ALTER TABLE foo ADD FOREIGN KEY fkoo ... NOT VALID;
>>
>> ALTER TABLE foo VALIDATE CONSTRAINT fkoo;
>
> This patch, which seems to be the latest version, no longer applies,
> and has not been updated based on the previous provided review
> comments.
>
> Also, this diff hunk looks scary to me:
>
> + if (must_use_query)
> + ereport(ERROR,
> + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
> + errmsg("cannot SELECT from primary
> key of relation \"%s\"",
> + RelationGetRelationName(rel))));
> +
>
> What's the justification for that?
Since this patch was reviewed on January 23rd by Marko Tiikkaja and
more briefly on February 3rd by me, and has not been updated, I am
marking it Returned with Feedback.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On 8 February 2011 03:50, Robert Haas <robertmhaas@gmail.com> wrote:
> On Thu, Feb 3, 2011 at 11:00 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>> On Fri, Jan 14, 2011 at 6:15 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
>>> Patch to implement the proposed feature attached, for CFJan2011.
>>>
>>> 2 sub-command changes:
>>>
>>> ALTER TABLE foo ADD FOREIGN KEY fkoo ... NOT VALID;
>>>
>>> ALTER TABLE foo VALIDATE CONSTRAINT fkoo;
>>
>> This patch, which seems to be the latest version, no longer applies,
>> and has not been updated based on the previous provided review
>> comments.
>>
>> Also, this diff hunk looks scary to me:
>>
>> + if (must_use_query)
>> + ereport(ERROR,
>> + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
>> + errmsg("cannot SELECT from primary
>> key of relation \"%s\"",
>> + RelationGetRelationName(rel))));
>> +
>>
>> What's the justification for that?
>
> Since this patch was reviewed on January 23rd by Marko Tiikkaja and
> more briefly on February 3rd by me, and has not been updated, I am
> marking it Returned with Feedback.
Just a note that since Alvaro created a patch to provide similar
functionality for constraints, I identified an issue with database
dumps, which apparently affects invalid foreign keys too:
http://archives.postgresql.org/pgsql-hackers/2011-06/msg00162.php
In other words, a database containing foreign keys that hasn't yet
been validated will not produce a dump containing the necessary NOT
VALID parameters. This would be fixed by Alvaro's patch.
--
Thom Brown
Twitter: @darkixion
IRC (freenode): dark_ixion
Registered Linux user: #516935
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Thu, Jun 2, 2011 at 5:34 PM, Thom Brown <thom@linux.com> wrote:
> On 8 February 2011 03:50, Robert Haas <robertmhaas@gmail.com> wrote:
>> On Thu, Feb 3, 2011 at 11:00 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>>> On Fri, Jan 14, 2011 at 6:15 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
>>>> Patch to implement the proposed feature attached, for CFJan2011.
>>>>
>>>> 2 sub-command changes:
>>>>
>>>> ALTER TABLE foo ADD FOREIGN KEY fkoo ... NOT VALID;
>>>>
>>>> ALTER TABLE foo VALIDATE CONSTRAINT fkoo;
>>>
>>> This patch, which seems to be the latest version, no longer applies,
>>> and has not been updated based on the previous provided review
>>> comments.
>>>
>>> Also, this diff hunk looks scary to me:
>>>
>>> + if (must_use_query)
>>> + ereport(ERROR,
>>> + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
>>> + errmsg("cannot SELECT from primary
>>> key of relation \"%s\"",
>>> + RelationGetRelationName(rel))));
>>> +
>>>
>>> What's the justification for that?
>>
>> Since this patch was reviewed on January 23rd by Marko Tiikkaja and
>> more briefly on February 3rd by me, and has not been updated, I am
>> marking it Returned with Feedback.
>
> Just a note that since Alvaro created a patch to provide similar
> functionality for constraints, I identified an issue with database
> dumps, which apparently affects invalid foreign keys too:
> http://archives.postgresql.org/pgsql-hackers/2011-06/msg00162.php
>
> In other words, a database containing foreign keys that hasn't yet
> been validated will not produce a dump containing the necessary NOT
> VALID parameters. This would be fixed by Alvaro's patch.
Sounds like someone needs to extract and apply that portion of
Alvaro's patch. I've added this to the open items list.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Excerpts from Robert Haas's message of vie jun 03 09:34:03 -0400 2011: > > Just a note that since Alvaro created a patch to provide similar > > functionality for constraints, I identified an issue with database > > dumps, which apparently affects invalid foreign keys too: > > http://archives.postgresql.org/pgsql-hackers/2011-06/msg00162.php > > > > In other words, a database containing foreign keys that hasn't yet > > been validated will not produce a dump containing the necessary NOT > > VALID parameters. This would be fixed by Alvaro's patch. > > Sounds like someone needs to extract and apply that portion of > Alvaro's patch. I've added this to the open items list. It's already a separate patch; I'll apply soon. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support