mirrors/postgres
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix pg_dump for better handling of inherited columns.

Browse Source 
Revise pg_dump's handling of inherited columns, which was last looked at
seriously in 2001, to eliminate several misbehaviors associated with
inherited default expressions and NOT NULL flags.  In particular make sure
that a column is printed in a child table's CREATE TABLE command if and
only if it has attislocal = true; the former behavior would sometimes cause
a column to become marked attislocal when it was not so marked in the
source database.  Also, stop relying on textual comparison of default
expressions to decide if they're inherited; instead, don't use
default-expression inheritance at all, but just install the default
explicitly at each level of the hierarchy.  This fixes the
search-path-related misbehavior recently exhibited by Chester Young, and
also removes some dubious assumptions about the order in which ALTER TABLE
SET DEFAULT commands would be executed.

Back-patch to all supported branches.
This commit is contained in:
Tom Lane 2012-02-10 13:28:25 -05:00
parent 7b4fcabb2f
commit 0951f4d1f5
3 changed files with 148 additions and 151 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

155
src/bin/pg_dump/common.c
View File

@ -269,7 +269,13 @@ flagInhTables(TableInfo *tblinfo, int numTables,
/* flagInhAttrs -
* for each dumpable table in tblinfo, flag its inherited attributes
* so when we dump the table out, we don't dump out the inherited attributes
*
* What we need to do here is detect child columns that inherit NOT NULL
* bits from their parents (so that we needn't specify that again for the
* child) and child columns that have DEFAULT NULL when their parents had
* some non-null default. In the latter case, we make up a dummy AttrDefInfo
* object so that we'll correctly emit the necessary DEFAULT NULL clause;
* otherwise the backend will apply an inherited default to the column.
*
* modifies tblinfo
*/
@ -285,7 +291,6 @@ flagInhAttrs(TableInfo *tblinfo, int numTables)
TableInfo *tbinfo = &(tblinfo[i]);
int numParents;
TableInfo **parents;
TableInfo *parent;
/* Sequences and views never have parents */
if (tbinfo->relkind == RELKIND_SEQUENCE ||
@ -302,132 +307,70 @@ flagInhAttrs(TableInfo *tblinfo, int numTables)
if (numParents == 0)
continue; /* nothing to see here, move along */
/*----------------------------------------------------------------
* For each attr, check the parent info: if no parent has an attr
* with the same name, then it's not inherited. If there *is* an
* attr with the same name, then only dump it if:
*
* - it is NOT NULL and zero parents are NOT NULL
* OR
* - it has a default value AND the default value does not match
* all parent default values, or no parents specify a default.
*
* See discussion on -hackers around 2-Apr-2001.
*----------------------------------------------------------------
*/
/* For each column, search for matching column names in parent(s) */
for (j = 0; j < tbinfo->numatts; j++)
{
bool foundAttr; /* Attr was found in a parent */
bool foundNotNull; /* Attr was NOT NULL in a parent */
bool defaultsMatch; /* All non-empty defaults match */
bool defaultsFound; /* Found a default in a parent */
AttrDefInfo *attrDef;
bool foundDefault; /* Found a default in a parent */
/* no point in examining dropped columns */
if (tbinfo->attisdropped[j])
continue;
foundAttr = false;
foundNotNull = false;
defaultsMatch = true;
defaultsFound = false;
attrDef = tbinfo->attrdefs[j];
foundDefault = false;
for (k = 0; k < numParents; k++)
{
TableInfo *parent = parents[k];
int inhAttrInd;
parent = parents[k];
inhAttrInd = strInArray(tbinfo->attnames[j],
parent->attnames,
parent->numatts);
if (inhAttrInd != -1)
if (inhAttrInd >= 0)
{
AttrDefInfo *inhDef = parent->attrdefs[inhAttrInd];
foundAttr = true;
foundNotNull |= parent->notnull[inhAttrInd];
if (inhDef != NULL)
{
defaultsFound = true;
/*
* If any parent has a default and the child doesn't,
* we have to emit an explicit DEFAULT NULL clause for
* the child, else the parent's default will win.
*/
if (attrDef == NULL)
{
attrDef = (AttrDefInfo *) malloc(sizeof(AttrDefInfo));
attrDef->dobj.objType = DO_ATTRDEF;
attrDef->dobj.catId.tableoid = 0;
attrDef->dobj.catId.oid = 0;
AssignDumpId(&attrDef->dobj);
attrDef->adtable = tbinfo;
attrDef->adnum = j + 1;
attrDef->adef_expr = strdup("NULL");
attrDef->dobj.name = strdup(tbinfo->dobj.name);
attrDef->dobj.namespace = tbinfo->dobj.namespace;
attrDef->dobj.dump = tbinfo->dobj.dump;
attrDef->separate = false;
addObjectDependency(&tbinfo->dobj,
attrDef->dobj.dumpId);
tbinfo->attrdefs[j] = attrDef;
}
if (strcmp(attrDef->adef_expr, inhDef->adef_expr) != 0)
{
defaultsMatch = false;
/*
* Whenever there is a non-matching parent
* default, add a dependency to force the parent
* default to be dumped first, in case the
* defaults end up being dumped as separate
* commands. Otherwise the parent default will
* override the child's when it is applied.
*/
addObjectDependency(&attrDef->dobj,
inhDef->dobj.dumpId);
}
}
foundDefault |= (parent->attrdefs[inhAttrInd] != NULL);
}
}
/*
* Based on the scan of the parents, decide if we can rely on the
* inherited attr
*/
if (foundAttr) /* Attr was inherited */
/* Remember if we found inherited NOT NULL */
tbinfo->inhNotNull[j] = foundNotNull;
/* Manufacture a DEFAULT NULL clause if necessary */
if (foundDefault && tbinfo->attrdefs[j] == NULL)
{
/* Set inherited flag by default */
tbinfo->inhAttrs[j] = true;
tbinfo->inhAttrDef[j] = true;
tbinfo->inhNotNull[j] = true;
AttrDefInfo *attrDef;
/*
* Clear it if attr had a default, but parents did not, or
* mismatch
*/
if ((attrDef != NULL) && (!defaultsFound || !defaultsMatch))
attrDef = (AttrDefInfo *) malloc(sizeof(AttrDefInfo));
attrDef->dobj.objType = DO_ATTRDEF;
attrDef->dobj.catId.tableoid = 0;
attrDef->dobj.catId.oid = 0;
AssignDumpId(&attrDef->dobj);
attrDef->dobj.name = strdup(tbinfo->dobj.name);
attrDef->dobj.namespace = tbinfo->dobj.namespace;
attrDef->dobj.dump = tbinfo->dobj.dump;
attrDef->adtable = tbinfo;
attrDef->adnum = j + 1;
attrDef->adef_expr = strdup("NULL");
/* Will column be dumped explicitly? */
if (shouldPrintColumn(tbinfo, j))
{
tbinfo->inhAttrs[j] = false;
tbinfo->inhAttrDef[j] = false;
attrDef->separate = false;
/* No dependency needed: NULL cannot have dependencies */
}
else
{
/* column will be suppressed, print default separately */
attrDef->separate = true;
/* ensure it comes out after the table */
addObjectDependency(&attrDef->dobj,
tbinfo->dobj.dumpId);
}
/*
* Clear it if NOT NULL and none of the parents were NOT NULL
*/
if (tbinfo->notnull[j] && !foundNotNull)
{
tbinfo->inhAttrs[j] = false;
tbinfo->inhNotNull[j] = false;
}
/* Clear it if attr has local definition */
if (tbinfo->attislocal[j])
tbinfo->inhAttrs[j] = false;
tbinfo->attrdefs[j] = attrDef;
}
}
}

129
src/bin/pg_dump/pg_dump.c
View File

@ -4796,6 +4796,9 @@ getTableAttrs(TableInfo *tblinfo, int numTables)
* attstattarget doesn't exist in 7.1. It does exist in 7.2, but
* we don't dump it because we can't tell whether it's been
* explicitly set or was just a default.
*
* attislocal doesn't exist before 7.3, either; in older databases
* we just assume that inherited columns had no local definition.
*/
appendPQExpBuffer(q, "SELECT a.attnum, a.attname, "
"a.atttypmod, -1 AS attstattarget, a.attstorage, "
@ -4858,10 +4861,8 @@ getTableAttrs(TableInfo *tblinfo, int numTables)
tbinfo->attalign = (char *) malloc(ntups * sizeof(char));
tbinfo->attislocal = (bool *) malloc(ntups * sizeof(bool));
tbinfo->notnull = (bool *) malloc(ntups * sizeof(bool));
tbinfo->attrdefs = (AttrDefInfo **) malloc(ntups * sizeof(AttrDefInfo *));
tbinfo->inhAttrs = (bool *) malloc(ntups * sizeof(bool));
tbinfo->inhAttrDef = (bool *) malloc(ntups * sizeof(bool));
tbinfo->inhNotNull = (bool *) malloc(ntups * sizeof(bool));
tbinfo->attrdefs = (AttrDefInfo **) malloc(ntups * sizeof(AttrDefInfo *));
hasdefaults = false;
for (j = 0; j < ntups; j++)
@ -4887,8 +4888,6 @@ getTableAttrs(TableInfo *tblinfo, int numTables)
if (PQgetvalue(res, j, i_atthasdef)[0] == 't')
hasdefaults = true;
/* these flags will be set in flagInhAttrs() */
tbinfo->inhAttrs[j] = false;
tbinfo->inhAttrDef[j] = false;
tbinfo->inhNotNull[j] = false;
}
@ -4952,12 +4951,28 @@ getTableAttrs(TableInfo *tblinfo, int numTables)
{
int adnum;
adnum = atoi(PQgetvalue(res, j, 2));
if (adnum <= 0 || adnum > ntups)
{
write_msg(NULL, "invalid adnum value %d for table \"%s\"\n",
adnum, tbinfo->dobj.name);
exit_nicely();
}
/*
* dropped columns shouldn't have defaults, but just in case,
* ignore 'em
*/
if (tbinfo->attisdropped[adnum - 1])
continue;
attrdefs[j].dobj.objType = DO_ATTRDEF;
attrdefs[j].dobj.catId.tableoid = atooid(PQgetvalue(res, j, 0));
attrdefs[j].dobj.catId.oid = atooid(PQgetvalue(res, j, 1));
AssignDumpId(&attrdefs[j].dobj);
attrdefs[j].adtable = tbinfo;
attrdefs[j].adnum = adnum = atoi(PQgetvalue(res, j, 2));
attrdefs[j].adnum = adnum;
attrdefs[j].adef_expr = strdup(PQgetvalue(res, j, 3));
attrdefs[j].dobj.name = strdup(tbinfo->dobj.name);
@ -4968,9 +4983,8 @@ getTableAttrs(TableInfo *tblinfo, int numTables)
/*
* Defaults on a VIEW must always be dumped as separate ALTER
* TABLE commands. Defaults on regular tables are dumped as
* part of the CREATE TABLE if possible. To check if it's
* safe, we mark the default as needing to appear before the
* CREATE.
* part of the CREATE TABLE if possible, which it won't be
* if the column is not going to be emitted explicitly.
*/
if (tbinfo->relkind == RELKIND_VIEW)
{
@ -4979,19 +4993,27 @@ getTableAttrs(TableInfo *tblinfo, int numTables)
addObjectDependency(&attrdefs[j].dobj,
tbinfo->dobj.dumpId);
}
else if (!shouldPrintColumn(tbinfo, adnum - 1))
{
/* column will be suppressed, print default separately */
attrdefs[j].separate = true;
/* needed in case pre-7.3 DB: */
addObjectDependency(&attrdefs[j].dobj,
tbinfo->dobj.dumpId);
}
else
{
attrdefs[j].separate = false;
/*
* Mark the default as needing to appear before the table,
* so that any dependencies it has must be emitted before
* the CREATE TABLE. If this is not possible, we'll
* change to "separate" mode while sorting dependencies.
*/
addObjectDependency(&tbinfo->dobj,
attrdefs[j].dobj.dumpId);
}
if (adnum <= 0 || adnum > ntups)
{
write_msg(NULL, "invalid adnum value %d for table \"%s\"\n",
adnum, tbinfo->dobj.name);
exit_nicely();
}
tbinfo->attrdefs[adnum - 1] = &attrdefs[j];
}
PQclear(res);
@ -5138,6 +5160,28 @@ getTableAttrs(TableInfo *tblinfo, int numTables)
destroyPQExpBuffer(q);
}
/*
* Test whether a column should be printed as part of table's CREATE TABLE.
* Column number is zero-based.
*
* Normally this is always true, but it's false for dropped columns, as well
* as those that were inherited without any local definition. (If we print
* such a column it will mistakenly get pg_attribute.attislocal set to true.)
* However, in binary_upgrade mode, we must print all such columns anyway and
* fix the attislocal/attisdropped state later, so as to keep control of the
* physical column order.
*
* This function exists because there are scattered nonobvious places that
* must be kept in sync with this decision.
*/
bool
shouldPrintColumn(TableInfo *tbinfo, int colno)
{
if (binary_upgrade)
return true;
return (tbinfo->attislocal[colno] && !tbinfo->attisdropped[colno]);
}
/*
* getTSParsers:
@ -9876,11 +9920,11 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
for (j = 0; j < tbinfo->numatts; j++)
{
/*
* Normally, dump if it's one of the table's own attrs, and not
* dropped. But for binary upgrade, dump all the columns.
* Normally, dump if it's locally defined in this table, and not
* dropped. But for binary upgrade, we'll dump all the columns,
* and then fix up the dropped and nonlocal cases below.
*/
if ((!tbinfo->inhAttrs[j] && !tbinfo->attisdropped[j]) ||
binary_upgrade)
if (shouldPrintColumn(tbinfo, j))
{
/* Format properly if not first attr */
if (actual_atts > 0)
@ -9919,19 +9963,16 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
}
/*
* Default value --- suppress if inherited (except in
* binary-upgrade case, where we're not doing normal
* inheritance) or if it's to be printed separately.
* Default value --- suppress if to be printed separately.
*/
if (tbinfo->attrdefs[j] != NULL &&
(!tbinfo->inhAttrDef[j] || binary_upgrade) &&
!tbinfo->attrdefs[j]->separate)
appendPQExpBuffer(q, " DEFAULT %s",
tbinfo->attrdefs[j]->adef_expr);
/*
* Not Null constraint --- suppress if inherited, except
* in binary-upgrade case.
* Not Null constraint --- suppress if inherited, except in
* binary-upgrade case where that won't work.
*/
if (tbinfo->notnull[j] &&
(!tbinfo->inhNotNull[j] || binary_upgrade))
@ -10106,16 +10147,36 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
}
}
/* Loop dumping statistics and storage statements */
/*
* Dump additional per-column properties that we can't handle in the
* main CREATE TABLE command.
*/
for (j = 0; j < tbinfo->numatts; j++)
{
/* None of this applies to dropped columns */
if (tbinfo->attisdropped[j])
continue;
/*
* If we didn't dump the column definition explicitly above, and
* it is NOT NULL and did not inherit that property from a parent,
* we have to mark it separately.
*/
if (!shouldPrintColumn(tbinfo, j) &&
tbinfo->notnull[j] && !tbinfo->inhNotNull[j])
{
appendPQExpBuffer(q, "ALTER TABLE ONLY %s ",
fmtId(tbinfo->dobj.name));
appendPQExpBuffer(q, "ALTER COLUMN %s SET NOT NULL;\n",
fmtId(tbinfo->attnames[j]));
}
/*
* Dump per-column statistics information. We only issue an ALTER
* TABLE statement if the attstattarget entry for this column is
* non-negative (i.e. it's not the default value)
*/
if (tbinfo->attstattarget[j] >= 0 &&
!tbinfo->attisdropped[j])
if (tbinfo->attstattarget[j] >= 0)
{
appendPQExpBuffer(q, "ALTER TABLE ONLY %s ",
fmtId(tbinfo->dobj.name));
@ -10129,7 +10190,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
* Dump per-column storage information. The statement is only
* dumped if the storage has been changed from the type's default.
*/
if (!tbinfo->attisdropped[j] && tbinfo->attstorage[j] != tbinfo->typstorage[j])
if (tbinfo->attstorage[j] != tbinfo->typstorage[j])
{
switch (tbinfo->attstorage[j])
{
@ -10206,18 +10267,18 @@ dumpAttrDef(Archive *fout, AttrDefInfo *adinfo)
PQExpBuffer q;
PQExpBuffer delq;
/* Only print it if "separate" mode is selected */
if (!tbinfo->dobj.dump || !adinfo->separate || dataOnly)
/* Skip if table definition not to be dumped */
if (!tbinfo->dobj.dump || dataOnly)
return;
/* Don't print inherited defaults, either, except for binary upgrade */
if (tbinfo->inhAttrDef[adnum - 1] && !binary_upgrade)
/* Skip if not "separate"; it was dumped in the table's definition */
if (!adinfo->separate)
return;
q = createPQExpBuffer();
delq = createPQExpBuffer();
appendPQExpBuffer(q, "ALTER TABLE %s ",
appendPQExpBuffer(q, "ALTER TABLE ONLY %s ",
fmtId(tbinfo->dobj.name));
appendPQExpBuffer(q, "ALTER COLUMN %s SET DEFAULT %s;\n",
fmtId(tbinfo->attnames[adnum - 1]),

15
src/bin/pg_dump/pg_dump.h
View File

@ -251,17 +251,9 @@ typedef struct _tableInfo
int *attlen; /* attribute length, used by binary_upgrade */
char *attalign; /* attribute align, used by binary_upgrade */
bool *attislocal; /* true if attr has local definition */
/*
* Note: we need to store per-attribute notnull, default, and constraint
* stuff for all interesting tables so that we can tell which constraints
* were inherited.
*/
bool *notnull; /* Not null constraints on attributes */
struct _attrDefInfo **attrdefs; /* DEFAULT expressions */
bool *inhAttrs; /* true if each attribute is inherited */
bool *inhAttrDef; /* true if attr's default is inherited */
bool *notnull; /* NOT NULL constraints on attributes */
bool *inhNotNull; /* true if NOT NULL is inherited */
struct _attrDefInfo **attrdefs; /* DEFAULT expressions */
struct _constraintInfo *checkexprs; /* CHECK constraints */
/*
@ -274,7 +266,7 @@ typedef struct _tableInfo
typedef struct _attrDefInfo
{
DumpableObject dobj;
DumpableObject dobj; /* note: dobj.name is name of table */
TableInfo *adtable; /* link to table of attribute */
int adnum;
char *adef_expr; /* decompiled DEFAULT expression */
@ -505,6 +497,7 @@ extern void getTriggers(TableInfo tblinfo[], int numTables);
extern ProcLangInfo *getProcLangs(int *numProcLangs);
extern CastInfo *getCasts(int *numCasts);
extern void getTableAttrs(TableInfo *tbinfo, int numTables);
extern bool shouldPrintColumn(TableInfo *tbinfo, int colno);
extern TSParserInfo *getTSParsers(int *numTSParsers);
extern TSDictInfo *getTSDictionaries(int *numTSDicts);
extern TSTemplateInfo *getTSTemplates(int *numTSTemplates);