Patch: fix pg_dump for inherited defaults & not-null flags

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)postgreSQL(dot)org
Subject: Patch: fix pg_dump for inherited defaults & not-null flags
Date: 2012-02-09 23:21:58
Message-ID: 3413.1328829718@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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?

regards, tom lane

Attachment Content-Type Size
dump-inherited-columns-properly.patch text/x-patch 24.1 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2012-02-10 06:24:29 Re: psql tab completion for SELECT
Previous Message Dimitri Fontaine 2012-02-09 22:02:46 Re: psql tab completion for SELECT