Skip to content

Commit 00bc96b

Browse files
tglsfdctglsfdc
committed
Fix pg_dump for better handling of inherited columns.
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.
1 parent d06e2d2 commit 00bc96b

File tree

4 files changed

+168
-157
lines changed

4 files changed

+168
-157
lines changed

‎src/bin/pg_dump/common.c

Lines changed: 48 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,13 @@ flagInhTables(TableInfo *tblinfo, int numTables,
281281

282282
/* flagInhAttrs -
283283
* for each dumpable table in tblinfo, flag its inherited attributes
284-
* so when we dump the table out, we don't dump out the inherited attributes
284+
*
285+
* What we need to do here is detect child columns that inherit NOT NULL
286+
* bits from their parents (so that we needn't specify that again for the
287+
* child) and child columns that have DEFAULT NULL when their parents had
288+
* some non-null default. In the latter case, we make up a dummy AttrDefInfo
289+
* object so that we'll correctly emit the necessary DEFAULT NULL clause;
290+
* otherwise the backend will apply an inherited default to the column.
285291
*
286292
* modifies tblinfo
287293
*/
@@ -297,7 +303,6 @@ flagInhAttrs(TableInfo *tblinfo, int numTables)
297303
TableInfo *tbinfo = &(tblinfo[i]);
298304
int numParents;
299305
TableInfo **parents;
300-
TableInfo *parent;
301306

302307
/* Sequences and views never have parents */
303308
if (tbinfo->relkind == RELKIND_SEQUENCE ||
@@ -314,132 +319,70 @@ flagInhAttrs(TableInfo *tblinfo, int numTables)
314319
if (numParents == 0)
315320
continue; /* nothing to see here, move along */
316321

317-
/*----------------------------------------------------------------
318-
* For each attr, check the parent info: if no parent has an attr
319-
* with the same name, then it's not inherited. If there *is* an
320-
* attr with the same name, then only dump it if:
321-
*
322-
* - it is NOT NULL and zero parents are NOT NULL
323-
* OR
324-
* - it has a default value AND the default value does not match
325-
* all parent default values, or no parents specify a default.
326-
*
327-
* See discussion on -hackers around 2-Apr-2001.
328-
*----------------------------------------------------------------
329-
*/
322+
/* For each column, search for matching column names in parent(s) */
330323
for (j = 0; j < tbinfo->numatts; j++)
331324
{
332-
bool foundAttr; /* Attr was found in a parent */
333325
bool foundNotNull; /* Attr was NOT NULL in a parent */
334-
bool defaultsMatch; /* All non-empty defaults match */
335-
bool defaultsFound; /* Found a default in a parent */
336-
AttrDefInfo *attrDef;
337-
338-
foundAttr = false;
339-
foundNotNull = false;
340-
defaultsMatch = true;
341-
defaultsFound = false;
326+
bool foundDefault; /* Found a default in a parent */
342327

343-
attrDef = tbinfo->attrdefs[j];
328+
/* no point in examining dropped columns */
329+
if (tbinfo->attisdropped[j])
330+
continue;
344331

332+
foundNotNull = false;
333+
foundDefault = false;
345334
for (k = 0; k < numParents; k++)
346335
{
336+
TableInfo *parent = parents[k];
347337
int inhAttrInd;
348338

349-
parent = parents[k];
350339
inhAttrInd = strInArray(tbinfo->attnames[j],
351340
parent->attnames,
352341
parent->numatts);
353-
354-
if (inhAttrInd != -1)
342+
if (inhAttrInd >= 0)
355343
{
356-
AttrDefInfo *inhDef = parent->attrdefs[inhAttrInd];
357-
358-
foundAttr = true;
359344
foundNotNull |= parent->notnull[inhAttrInd];
360-
if (inhDef != NULL)
361-
{
362-
defaultsFound = true;
363-
364-
/*
365-
* If any parent has a default and the child doesn't,
366-
* we have to emit an explicit DEFAULT NULL clause for
367-
* the child, else the parent's default will win.
368-
*/
369-
if (attrDef == NULL)
370-
{
371-
attrDef = (AttrDefInfo *) pg_malloc(sizeof(AttrDefInfo));
372-
attrDef->dobj.objType = DO_ATTRDEF;
373-
attrDef->dobj.catId.tableoid = 0;
374-
attrDef->dobj.catId.oid = 0;
375-
AssignDumpId(&attrDef->dobj);
376-
attrDef->adtable = tbinfo;
377-
attrDef->adnum = j + 1;
378-
attrDef->adef_expr = pg_strdup("NULL");
379-
380-
attrDef->dobj.name = pg_strdup(tbinfo->dobj.name);
381-
attrDef->dobj.namespace = tbinfo->dobj.namespace;
382-
383-
attrDef->dobj.dump = tbinfo->dobj.dump;
384-
385-
attrDef->separate = false;
386-
addObjectDependency(&tbinfo->dobj,
387-
attrDef->dobj.dumpId);
388-
389-
tbinfo->attrdefs[j] = attrDef;
390-
}
391-
if (strcmp(attrDef->adef_expr, inhDef->adef_expr) != 0)
392-
{
393-
defaultsMatch = false;
394-
395-
/*
396-
* Whenever there is a non-matching parent
397-
* default, add a dependency to force the parent
398-
* default to be dumped first, in case the
399-
* defaults end up being dumped as separate
400-
* commands. Otherwise the parent default will
401-
* override the child's when it is applied.
402-
*/
403-
addObjectDependency(&attrDef->dobj,
404-
inhDef->dobj.dumpId);
405-
}
406-
}
345+
foundDefault |= (parent->attrdefs[inhAttrInd] != NULL);
407346
}
408347
}
409348

410-
/*
411-
* Based on the scan of the parents, decide if we can rely on the
412-
* inherited attr
413-
*/
414-
if (foundAttr) /* Attr was inherited */
349+
/* Remember if we found inherited NOT NULL */
350+
tbinfo->inhNotNull[j] = foundNotNull;
351+
352+
/* Manufacture a DEFAULT NULL clause if necessary */
353+
if (foundDefault && tbinfo->attrdefs[j] == NULL)
415354
{
416-
/* Set inherited flag by default */
417-
tbinfo->inhAttrs[j] = true;
418-
tbinfo->inhAttrDef[j] = true;
419-
tbinfo->inhNotNull[j] = true;
420-
421-
/*
422-
* Clear it if attr had a default, but parents did not, or
423-
* mismatch
424-
*/
425-
if ((attrDef != NULL) && (!defaultsFound || !defaultsMatch))
355+
AttrDefInfo *attrDef;
356+
357+
attrDef = (AttrDefInfo *) pg_malloc(sizeof(AttrDefInfo));
358+
attrDef->dobj.objType = DO_ATTRDEF;
359+
attrDef->dobj.catId.tableoid = 0;
360+
attrDef->dobj.catId.oid = 0;
361+
AssignDumpId(&attrDef->dobj);
362+
attrDef->dobj.name = pg_strdup(tbinfo->dobj.name);
363+
attrDef->dobj.namespace = tbinfo->dobj.namespace;
364+
attrDef->dobj.dump = tbinfo->dobj.dump;
365+
366+
attrDef->adtable = tbinfo;
367+
attrDef->adnum = j + 1;
368+
attrDef->adef_expr = pg_strdup("NULL");
369+
370+
/* Will column be dumped explicitly? */
371+
if (shouldPrintColumn(tbinfo, j))
426372
{
427-
tbinfo->inhAttrs[j] = false;
428-
tbinfo->inhAttrDef[j] = false;
373+
attrDef->separate = false;
374+
/* No dependency needed: NULL cannot have dependencies */
429375
}
430-
431-
/*
432-
* Clear it if NOT NULL and none of the parents were NOT NULL
433-
*/
434-
if (tbinfo->notnull[j] && !foundNotNull)
376+
else
435377
{
436-
tbinfo->inhAttrs[j] = false;
437-
tbinfo->inhNotNull[j] = false;
378+
/* column will be suppressed, print default separately */
379+
attrDef->separate = true;
380+
/* ensure it comes out after the table */
381+
addObjectDependency(&attrDef->dobj,
382+
tbinfo->dobj.dumpId);
438383
}
439384

440-
/* Clear it if attr has local definition */
441-
if (tbinfo->attislocal[j])
442-
tbinfo->inhAttrs[j] = false;
385+
tbinfo->attrdefs[j] = attrDef;
443386
}
444387
}
445388
}

0 commit comments

Comments
 (0)