Skip to content

Commit ab060c7

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 cc0f6ff commit ab060c7

File tree

4 files changed

+164
-154
lines changed

4 files changed

+164
-154
lines changed

‎src/bin/pg_dump/common.c

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

276276
/* flagInhAttrs -
277277
* for each dumpable table in tblinfo, flag its inherited attributes
278-
* so when we dump the table out, we don't dump out the inherited attributes
278+
*
279+
* What we need to do here is detect child columns that inherit NOT NULL
280+
* bits from their parents (so that we needn't specify that again for the
281+
* child) and child columns that have DEFAULT NULL when their parents had
282+
* some non-null default. In the latter case, we make up a dummy AttrDefInfo
283+
* object so that we'll correctly emit the necessary DEFAULT NULL clause;
284+
* otherwise the backend will apply an inherited default to the column.
279285
*
280286
* modifies tblinfo
281287
*/
@@ -291,7 +297,6 @@ flagInhAttrs(TableInfo *tblinfo, int numTables)
291297
TableInfo *tbinfo = &(tblinfo[i]);
292298
int numParents;
293299
TableInfo **parents;
294-
TableInfo *parent;
295300

296301
/* Sequences and views never have parents */
297302
if (tbinfo->relkind == RELKIND_SEQUENCE ||
@@ -308,132 +313,70 @@ flagInhAttrs(TableInfo *tblinfo, int numTables)
308313
if (numParents == 0)
309314
continue; /* nothing to see here, move along */
310315

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

337-
attrDef = tbinfo->attrdefs[j];
322+
/* no point in examining dropped columns */
323+
if (tbinfo->attisdropped[j])
324+
continue;
338325

326+
foundNotNull = false;
327+
foundDefault = false;
339328
for (k = 0; k < numParents; k++)
340329
{
330+
TableInfo *parent = parents[k];
341331
int inhAttrInd;
342332

343-
parent = parents[k];
344333
inhAttrInd = strInArray(tbinfo->attnames[j],
345334
parent->attnames,
346335
parent->numatts);
347-
348-
if (inhAttrInd != -1)
336+
if (inhAttrInd >= 0)
349337
{
350-
AttrDefInfo *inhDef = parent->attrdefs[inhAttrInd];
351-
352-
foundAttr = true;
353338
foundNotNull |= parent->notnull[inhAttrInd];
354-
if (inhDef != NULL)
355-
{
356-
defaultsFound = true;
357-
358-
/*
359-
* If any parent has a default and the child doesn't,
360-
* we have to emit an explicit DEFAULT NULL clause for
361-
* the child, else the parent's default will win.
362-
*/
363-
if (attrDef == NULL)
364-
{
365-
attrDef = (AttrDefInfo *) malloc(sizeof(AttrDefInfo));
366-
attrDef->dobj.objType = DO_ATTRDEF;
367-
attrDef->dobj.catId.tableoid = 0;
368-
attrDef->dobj.catId.oid = 0;
369-
AssignDumpId(&attrDef->dobj);
370-
attrDef->adtable = tbinfo;
371-
attrDef->adnum = j + 1;
372-
attrDef->adef_expr = strdup("NULL");
373-
374-
attrDef->dobj.name = strdup(tbinfo->dobj.name);
375-
attrDef->dobj.namespace = tbinfo->dobj.namespace;
376-
377-
attrDef->dobj.dump = tbinfo->dobj.dump;
378-
379-
attrDef->separate = false;
380-
addObjectDependency(&tbinfo->dobj,
381-
attrDef->dobj.dumpId);
382-
383-
tbinfo->attrdefs[j] = attrDef;
384-
}
385-
if (strcmp(attrDef->adef_expr, inhDef->adef_expr) != 0)
386-
{
387-
defaultsMatch = false;
388-
389-
/*
390-
* Whenever there is a non-matching parent
391-
* default, add a dependency to force the parent
392-
* default to be dumped first, in case the
393-
* defaults end up being dumped as separate
394-
* commands. Otherwise the parent default will
395-
* override the child's when it is applied.
396-
*/
397-
addObjectDependency(&attrDef->dobj,
398-
inhDef->dobj.dumpId);
399-
}
400-
}
339+
foundDefault |= (parent->attrdefs[inhAttrInd] != NULL);
401340
}
402341
}
403342

404-
/*
405-
* Based on the scan of the parents, decide if we can rely on the
406-
* inherited attr
407-
*/
408-
if (foundAttr) /* Attr was inherited */
343+
/* Remember if we found inherited NOT NULL */
344+
tbinfo->inhNotNull[j] = foundNotNull;
345+
346+
/* Manufacture a DEFAULT NULL clause if necessary */
347+
if (foundDefault && tbinfo->attrdefs[j] == NULL)
409348
{
410-
/* Set inherited flag by default */
411-
tbinfo->inhAttrs[j] = true;
412-
tbinfo->inhAttrDef[j] = true;
413-
tbinfo->inhNotNull[j] = true;
414-
415-
/*
416-
* Clear it if attr had a default, but parents did not, or
417-
* mismatch
418-
*/
419-
if ((attrDef != NULL) && (!defaultsFound || !defaultsMatch))
349+
AttrDefInfo *attrDef;
350+
351+
attrDef = (AttrDefInfo *) malloc(sizeof(AttrDefInfo));
352+
attrDef->dobj.objType = DO_ATTRDEF;
353+
attrDef->dobj.catId.tableoid = 0;
354+
attrDef->dobj.catId.oid = 0;
355+
AssignDumpId(&attrDef->dobj);
356+
attrDef->dobj.name = strdup(tbinfo->dobj.name);
357+
attrDef->dobj.namespace = tbinfo->dobj.namespace;
358+
attrDef->dobj.dump = tbinfo->dobj.dump;
359+
360+
attrDef->adtable = tbinfo;
361+
attrDef->adnum = j + 1;
362+
attrDef->adef_expr = strdup("NULL");
363+
364+
/* Will column be dumped explicitly? */
365+
if (shouldPrintColumn(tbinfo, j))
420366
{
421-
tbinfo->inhAttrs[j] = false;
422-
tbinfo->inhAttrDef[j] = false;
367+
attrDef->separate = false;
368+
/* No dependency needed: NULL cannot have dependencies */
423369
}
424-
425-
/*
426-
* Clear it if NOT NULL and none of the parents were NOT NULL
427-
*/
428-
if (tbinfo->notnull[j] && !foundNotNull)
370+
else
429371
{
430-
tbinfo->inhAttrs[j] = false;
431-
tbinfo->inhNotNull[j] = false;
372+
/* column will be suppressed, print default separately */
373+
attrDef->separate = true;
374+
/* ensure it comes out after the table */
375+
addObjectDependency(&attrDef->dobj,
376+
tbinfo->dobj.dumpId);
432377
}
433378

434-
/* Clear it if attr has local definition */
435-
if (tbinfo->attislocal[j])
436-
tbinfo->inhAttrs[j] = false;
379+
tbinfo->attrdefs[j] = attrDef;
437380
}
438381
}
439382
}

0 commit comments

Comments
 (0)