On Thu, Feb 9, 2012 at 6:21 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: > Attached is a proposed patch to deal with the issue described here: > http://archives.postgresql.org/pgsql-bugs/2012-02/msg00000.php > > Even though we'd previously realized that comparing the text of > inherited CHECK expressions is an entirely unsafe way to detect > expression equivalence (cf comments for guessConstraintInheritance), > pg_dump is still doing that for inherited DEFAULT expressions, with > the predictable result that it does the wrong thing in this sort > of example. > > Furthermore, as I looked more closely at the code, I realized that > there is another pretty fundamental issue: if an inherited column has a > default expression or NOT NULL bit that it did not inherit from its > parent, flagInhAttrs forces the column to be treated as non-inherited, > so that it will be emitted as part of the child table's CREATE TABLE > command. This is *wrong* if the column is not attislocal; it will > result in the column incorrectly having the attislocal property after > restore. (Note: such a situation could only arise if the user had > altered the column's default or NOT NULL property with ALTER TABLE after > creation.) > > All of this logic predates the invention of attislocal, and really is > attempting to make up for the lack of that bit, so it's not all that > surprising that it falls down. > > So the attached patch makes the emit-column-or-not behavior depend only > on attislocal (except for binary upgrade which has its own kluge > solution for the problem; though note that the whether-to-dump tests now > exactly match the special cases for binary upgrade, which they did not > before). Also, I've dropped the former attempts to exploit inheritance > of defaults, and so the code will now emit a default explicitly for each > child in an inheritance hierarchy, even if it didn't really need to. > Since the backend doesn't track whether defaults are inherited, this > doesn't cause any failure to restore the catalog state properly. > > Although this is a bug fix, it's a nontrivial change in the logic and > so I'm hesitant to back-patch into stable branches. Given the lack of > prior complaints, maybe it would be best to leave it unfixed in existing > branches? Not sure. Thoughts?
I guess I'd be in favor of back-patching it, if that doesn't look like too much of a job. We shouldn't assume that because only one person reports a problem, no one else has been or will be affected. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
