Skip to content

Commit 0bf8364

Browse files
peterepetere
committed
pg_dump: Fix dumping of inherited generated columns
Generation expressions of generated columns are always inherited, so there is no need to set them separately in child tables, and there is no syntax to do so either. The code previously used the code paths for the handling of default values, for which different rules apply; in particular it might want to set a default value explicitly for an inherited column. This resulted in unrestorable dumps. For generated columns, just skip them in inherited tables. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/15830.1575468847%40sss.pgh.pa.us
1 parent ef3d461 commit 0bf8364

File tree

3 files changed

+102
-12
lines changed

3 files changed

+102
-12
lines changed

‎src/bin/pg_dump/common.c

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -467,12 +467,22 @@ flagInhIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
467467
/* flagInhAttrs -
468468
* for each dumpable table in tblinfo, flag its inherited attributes
469469
*
470-
* What we need to do here is detect child columns that inherit NOT NULL
471-
* bits from their parents (so that we needn't specify that again for the
472-
* child) and child columns that have DEFAULT NULL when their parents had
473-
* some non-null default. In the latter case, we make up a dummy AttrDefInfo
474-
* object so that we'll correctly emit the necessary DEFAULT NULL clause;
475-
* otherwise the backend will apply an inherited default to the column.
470+
* What we need to do here is:
471+
*
472+
* - Detect child columns that inherit NOT NULL bits from their parents, so
473+
* that we needn't specify that again for the child.
474+
*
475+
* - Detect child columns that have DEFAULT NULL when their parents had some
476+
* non-null default. In this case, we make up a dummy AttrDefInfo object so
477+
* that we'll correctly emit the necessary DEFAULT NULL clause; otherwise
478+
* the backend will apply an inherited default to the column.
479+
*
480+
* - Detect child columns that have a generation expression when their parents
481+
* also have one. Generation expressions are always inherited, so there is
482+
* no need to set them again in child tables, and there is no syntax for it
483+
* either. (Exception: In binary upgrade mode we dump them because
484+
* inherited tables are recreated standalone first and then reattached to
485+
* the parent.)
476486
*
477487
* modifies tblinfo
478488
*/
@@ -510,13 +520,15 @@ flagInhAttrs(DumpOptions *dopt, TableInfo *tblinfo, int numTables)
510520
{
511521
bool foundNotNull; /* Attr was NOT NULL in a parent */
512522
bool foundDefault; /* Found a default in a parent */
523+
bool foundGenerated; /* Found a generated in a parent */
513524

514525
/* no point in examining dropped columns */
515526
if (tbinfo->attisdropped[j])
516527
continue;
517528

518529
foundNotNull = false;
519530
foundDefault = false;
531+
foundGenerated = false;
520532
for (k = 0; k < numParents; k++)
521533
{
522534
TableInfo *parent = parents[k];
@@ -528,7 +540,8 @@ flagInhAttrs(DumpOptions *dopt, TableInfo *tblinfo, int numTables)
528540
if (inhAttrInd >= 0)
529541
{
530542
foundNotNull |= parent->notnull[inhAttrInd];
531-
foundDefault |= (parent->attrdefs[inhAttrInd] != NULL);
543+
foundDefault |= (parent->attrdefs[inhAttrInd] != NULL && !parent->attgenerated[inhAttrInd]);
544+
foundGenerated |= parent->attgenerated[inhAttrInd];
532545
}
533546
}
534547

@@ -570,6 +583,10 @@ flagInhAttrs(DumpOptions *dopt, TableInfo *tblinfo, int numTables)
570583

571584
tbinfo->attrdefs[j] = attrDef;
572585
}
586+
587+
/* Remove generation expression from child */
588+
if (foundGenerated && !dopt->binary_upgrade)
589+
tbinfo->attrdefs[j] = NULL;
573590
}
574591
}
575592
}

‎src/bin/pg_dump/pg_dump.c

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8839,13 +8839,37 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
88398839
attrdefs[j].dobj.dump = tbinfo->dobj.dump;
88408840

88418841
/*
8842-
* Defaults on a VIEW must always be dumped as separate ALTER
8843-
* TABLE commands. Defaults on regular tables are dumped as
8844-
* part of the CREATE TABLE if possible, which it won't be if
8845-
* the column is not going to be emitted explicitly.
8842+
* Figure out whether the default/generation expression should
8843+
* be dumped as part of the main CREATE TABLE (or similar)
8844+
* command or as a separate ALTER TABLE (or similar) command.
8845+
* The preference is to put it into the CREATE command, but in
8846+
* some cases that's not possible.
88468847
*/
8847-
if (tbinfo->relkind == RELKIND_VIEW)
8848+
if (tbinfo->attgenerated[adnum - 1])
88488849
{
8850+
/*
8851+
* Column generation expressions cannot be dumped
8852+
* separately, because there is no syntax for it. The
8853+
* !shouldPrintColumn case below will be tempted to set
8854+
* them to separate if they are attached to an inherited
8855+
* column without a local definition, but that would be
8856+
* wrong and unnecessary, because generation expressions
8857+
* are always inherited, so there is no need to set them
8858+
* again in child tables, and there is no syntax for it
8859+
* either. By setting separate to false here we prevent
8860+
* the "default" from being processed as its own dumpable
8861+
* object, and flagInhAttrs() will remove it from the
8862+
* table when it detects that it belongs to an inherited
8863+
* column.
8864+
*/
8865+
attrdefs[j].separate = false;
8866+
}
8867+
else if (tbinfo->relkind == RELKIND_VIEW)
8868+
{
8869+
/*
8870+
* Defaults on a VIEW must always be dumped as separate
8871+
* ALTER TABLE commands.
8872+
*/
88498873
attrdefs[j].separate = true;
88508874
}
88518875
else if (!shouldPrintColumn(dopt, tbinfo, adnum - 1))
@@ -8856,7 +8880,10 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
88568880
else
88578881
{
88588882
attrdefs[j].separate = false;
8883+
}
88598884

8885+
if (!attrdefs[j].separate)
8886+
{
88608887
/*
88618888
* Mark the default as needing to appear before the table,
88628889
* so that any dependencies it has must be emitted before

‎src/bin/pg_dump/t/002_pg_dump.pl

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2493,6 +2493,52 @@
24932493
unlike => { exclude_dump_test_schema => 1, },
24942494
},
24952495
2496+
'CREATE TABLE test_table_generated_child1 (without local columns)' => {
2497+
create_order => 4,
2498+
create_sql => 'CREATE TABLE dump_test.test_table_generated_child1 ()
2499+
INHERITS (dump_test.test_table_generated);',
2500+
regexp => qr/^
2501+
\QCREATE TABLE dump_test.test_table_generated_child1 (\E\n
2502+
\)\n
2503+
\QINHERITS (dump_test.test_table_generated);\E\n
2504+
/xms,
2505+
like =>
2506+
{ %full_runs, %dump_test_schema_runs, section_pre_data => 1, },
2507+
unlike => {
2508+
binary_upgrade => 1,
2509+
exclude_dump_test_schema => 1,
2510+
},
2511+
},
2512+
2513+
'ALTER TABLE test_table_generated_child1' => {
2514+
regexp =>
2515+
qr/^\QALTER TABLE ONLY dump_test.test_table_generated_child1 ALTER COLUMN col2 \E/m,
2516+
2517+
# should not get emitted
2518+
like => {},
2519+
},
2520+
2521+
'CREATE TABLE test_table_generated_child2 (with local columns)' => {
2522+
create_order => 4,
2523+
create_sql => 'CREATE TABLE dump_test.test_table_generated_child2 (
2524+
col1 int,
2525+
col2 int
2526+
) INHERITS (dump_test.test_table_generated);',
2527+
regexp => qr/^
2528+
\QCREATE TABLE dump_test.test_table_generated_child2 (\E\n
2529+
\s+\Qcol1 integer,\E\n
2530+
\s+\Qcol2 integer\E\n
2531+
\)\n
2532+
\QINHERITS (dump_test.test_table_generated);\E\n
2533+
/xms,
2534+
like =>
2535+
{ %full_runs, %dump_test_schema_runs, section_pre_data => 1, },
2536+
unlike => {
2537+
binary_upgrade => 1,
2538+
exclude_dump_test_schema => 1,
2539+
},
2540+
},
2541+
24962542
'CREATE TABLE table_with_stats' => {
24972543
create_order => 98,
24982544
create_sql => 'CREATE TABLE dump_test.table_index_stats (

0 commit comments

Comments
 (0)