Skip to content

Commit 1dd6baf

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 78fab84 commit 1dd6baf

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
@@ -433,12 +433,22 @@ flagInhIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
433433
/* flagInhAttrs -
434434
* for each dumpable table in tblinfo, flag its inherited attributes
435435
*
436-
* What we need to do here is detect child columns that inherit NOT NULL
437-
* bits from their parents (so that we needn't specify that again for the
438-
* child) and child columns that have DEFAULT NULL when their parents had
439-
* some non-null default. In the latter case, we make up a dummy AttrDefInfo
440-
* object so that we'll correctly emit the necessary DEFAULT NULL clause;
441-
* otherwise the backend will apply an inherited default to the column.
436+
* What we need to do here is:
437+
*
438+
* - Detect child columns that inherit NOT NULL bits from their parents, so
439+
* that we needn't specify that again for the child.
440+
*
441+
* - Detect child columns that have DEFAULT NULL when their parents had some
442+
* non-null default. In this case, we make up a dummy AttrDefInfo object so
443+
* that we'll correctly emit the necessary DEFAULT NULL clause; otherwise
444+
* the backend will apply an inherited default to the column.
445+
*
446+
* - Detect child columns that have a generation expression when their parents
447+
* also have one. Generation expressions are always inherited, so there is
448+
* no need to set them again in child tables, and there is no syntax for it
449+
* either. (Exception: In binary upgrade mode we dump them because
450+
* inherited tables are recreated standalone first and then reattached to
451+
* the parent.)
442452
*
443453
* modifies tblinfo
444454
*/
@@ -476,13 +486,15 @@ flagInhAttrs(DumpOptions *dopt, TableInfo *tblinfo, int numTables)
476486
{
477487
bool foundNotNull; /* Attr was NOT NULL in a parent */
478488
bool foundDefault; /* Found a default in a parent */
489+
bool foundGenerated; /* Found a generated in a parent */
479490

480491
/* no point in examining dropped columns */
481492
if (tbinfo->attisdropped[j])
482493
continue;
483494

484495
foundNotNull = false;
485496
foundDefault = false;
497+
foundGenerated = false;
486498
for (k = 0; k < numParents; k++)
487499
{
488500
TableInfo *parent = parents[k];
@@ -494,7 +506,8 @@ flagInhAttrs(DumpOptions *dopt, TableInfo *tblinfo, int numTables)
494506
if (inhAttrInd >= 0)
495507
{
496508
foundNotNull |= parent->notnull[inhAttrInd];
497-
foundDefault |= (parent->attrdefs[inhAttrInd] != NULL);
509+
foundDefault |= (parent->attrdefs[inhAttrInd] != NULL && !parent->attgenerated[inhAttrInd]);
510+
foundGenerated |= parent->attgenerated[inhAttrInd];
498511
}
499512
}
500513

@@ -536,6 +549,10 @@ flagInhAttrs(DumpOptions *dopt, TableInfo *tblinfo, int numTables)
536549

537550
tbinfo->attrdefs[j] = attrDef;
538551
}
552+
553+
/* Remove generation expression from child */
554+
if (foundGenerated && !dopt->binary_upgrade)
555+
tbinfo->attrdefs[j] = NULL;
539556
}
540557
}
541558
}

‎src/bin/pg_dump/pg_dump.c

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

86188618
/*
8619-
* Defaults on a VIEW must always be dumped as separate ALTER
8620-
* TABLE commands. Defaults on regular tables are dumped as
8621-
* part of the CREATE TABLE if possible, which it won't be if
8622-
* the column is not going to be emitted explicitly.
8619+
* Figure out whether the default/generation expression should
8620+
* be dumped as part of the main CREATE TABLE (or similar)
8621+
* command or as a separate ALTER TABLE (or similar) command.
8622+
* The preference is to put it into the CREATE command, but in
8623+
* some cases that's not possible.
86238624
*/
8624-
if (tbinfo->relkind == RELKIND_VIEW)
8625+
if (tbinfo->attgenerated[adnum - 1])
86258626
{
8627+
/*
8628+
* Column generation expressions cannot be dumped
8629+
* separately, because there is no syntax for it. The
8630+
* !shouldPrintColumn case below will be tempted to set
8631+
* them to separate if they are attached to an inherited
8632+
* column without a local definition, but that would be
8633+
* wrong and unnecessary, because generation expressions
8634+
* are always inherited, so there is no need to set them
8635+
* again in child tables, and there is no syntax for it
8636+
* either. By setting separate to false here we prevent
8637+
* the "default" from being processed as its own dumpable
8638+
* object, and flagInhAttrs() will remove it from the
8639+
* table when it detects that it belongs to an inherited
8640+
* column.
8641+
*/
8642+
attrdefs[j].separate = false;
8643+
}
8644+
else if (tbinfo->relkind == RELKIND_VIEW)
8645+
{
8646+
/*
8647+
* Defaults on a VIEW must always be dumped as separate
8648+
* ALTER TABLE commands.
8649+
*/
86268650
attrdefs[j].separate = true;
86278651
}
86288652
else if (!shouldPrintColumn(dopt, tbinfo, adnum - 1))
@@ -8633,7 +8657,10 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
86338657
else
86348658
{
86358659
attrdefs[j].separate = false;
8660+
}
86368661

8662+
if (!attrdefs[j].separate)
8663+
{
86378664
/*
86388665
* Mark the default as needing to appear before the table,
86398666
* 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
@@ -2473,6 +2473,52 @@
24732473
unlike => { exclude_dump_test_schema => 1, },
24742474
},
24752475
2476+
'CREATE TABLE test_table_generated_child1 (without local columns)' => {
2477+
create_order => 4,
2478+
create_sql => 'CREATE TABLE dump_test.test_table_generated_child1 ()
2479+
INHERITS (dump_test.test_table_generated);',
2480+
regexp => qr/^
2481+
\QCREATE TABLE dump_test.test_table_generated_child1 (\E\n
2482+
\)\n
2483+
\QINHERITS (dump_test.test_table_generated);\E\n
2484+
/xms,
2485+
like =>
2486+
{ %full_runs, %dump_test_schema_runs, section_pre_data => 1, },
2487+
unlike => {
2488+
binary_upgrade => 1,
2489+
exclude_dump_test_schema => 1,
2490+
},
2491+
},
2492+
2493+
'ALTER TABLE test_table_generated_child1' => {
2494+
regexp =>
2495+
qr/^\QALTER TABLE ONLY dump_test.test_table_generated_child1 ALTER COLUMN col2 \E/m,
2496+
2497+
# should not get emitted
2498+
like => {},
2499+
},
2500+
2501+
'CREATE TABLE test_table_generated_child2 (with local columns)' => {
2502+
create_order => 4,
2503+
create_sql => 'CREATE TABLE dump_test.test_table_generated_child2 (
2504+
col1 int,
2505+
col2 int
2506+
) INHERITS (dump_test.test_table_generated);',
2507+
regexp => qr/^
2508+
\QCREATE TABLE dump_test.test_table_generated_child2 (\E\n
2509+
\s+\Qcol1 integer,\E\n
2510+
\s+\Qcol2 integer\E\n
2511+
\)\n
2512+
\QINHERITS (dump_test.test_table_generated);\E\n
2513+
/xms,
2514+
like =>
2515+
{ %full_runs, %dump_test_schema_runs, section_pre_data => 1, },
2516+
unlike => {
2517+
binary_upgrade => 1,
2518+
exclude_dump_test_schema => 1,
2519+
},
2520+
},
2521+
24762522
'CREATE TABLE table_with_stats' => {
24772523
create_order => 98,
24782524
create_sql => 'CREATE TABLE dump_test.table_index_stats (

0 commit comments

Comments
 (0)