Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 1d3ce02

Browse filesBrowse 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 0fe8b1f commit 1d3ce02
Copy full SHA for 1d3ce02

File tree

Expand file treeCollapse file tree

3 files changed

+102
-12
lines changed
Filter options
Expand file treeCollapse file tree

3 files changed

+102
-12
lines changed

‎src/bin/pg_dump/common.c

Copy file name to clipboardExpand all lines: src/bin/pg_dump/common.c
+24-7Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -431,12 +431,22 @@ flagInhIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
431431
/* flagInhAttrs -
432432
* for each dumpable table in tblinfo, flag its inherited attributes
433433
*
434-
* What we need to do here is detect child columns that inherit NOT NULL
435-
* bits from their parents (so that we needn't specify that again for the
436-
* child) and child columns that have DEFAULT NULL when their parents had
437-
* some non-null default. In the latter case, we make up a dummy AttrDefInfo
438-
* object so that we'll correctly emit the necessary DEFAULT NULL clause;
439-
* otherwise the backend will apply an inherited default to the column.
434+
* What we need to do here is:
435+
*
436+
* - Detect child columns that inherit NOT NULL bits from their parents, so
437+
* that we needn't specify that again for the child.
438+
*
439+
* - Detect child columns that have DEFAULT NULL when their parents had some
440+
* non-null default. In this case, we make up a dummy AttrDefInfo object so
441+
* that we'll correctly emit the necessary DEFAULT NULL clause; otherwise
442+
* the backend will apply an inherited default to the column.
443+
*
444+
* - Detect child columns that have a generation expression when their parents
445+
* also have one. Generation expressions are always inherited, so there is
446+
* no need to set them again in child tables, and there is no syntax for it
447+
* either. (Exception: In binary upgrade mode we dump them because
448+
* inherited tables are recreated standalone first and then reattached to
449+
* the parent.)
440450
*
441451
* modifies tblinfo
442452
*/
@@ -474,13 +484,15 @@ flagInhAttrs(DumpOptions *dopt, TableInfo *tblinfo, int numTables)
474484
{
475485
bool foundNotNull; /* Attr was NOT NULL in a parent */
476486
bool foundDefault; /* Found a default in a parent */
487+
bool foundGenerated; /* Found a generated in a parent */
477488

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

482493
foundNotNull = false;
483494
foundDefault = false;
495+
foundGenerated = false;
484496
for (k = 0; k < numParents; k++)
485497
{
486498
TableInfo *parent = parents[k];
@@ -492,7 +504,8 @@ flagInhAttrs(DumpOptions *dopt, TableInfo *tblinfo, int numTables)
492504
if (inhAttrInd >= 0)
493505
{
494506
foundNotNull |= parent->notnull[inhAttrInd];
495-
foundDefault |= (parent->attrdefs[inhAttrInd] != NULL);
507+
foundDefault |= (parent->attrdefs[inhAttrInd] != NULL && !parent->attgenerated[inhAttrInd]);
508+
foundGenerated |= parent->attgenerated[inhAttrInd];
496509
}
497510
}
498511

@@ -534,6 +547,10 @@ flagInhAttrs(DumpOptions *dopt, TableInfo *tblinfo, int numTables)
534547

535548
tbinfo->attrdefs[j] = attrDef;
536549
}
550+
551+
/* Remove generation expression from child */
552+
if (foundGenerated && !dopt->binary_upgrade)
553+
tbinfo->attrdefs[j] = NULL;
537554
}
538555
}
539556
}

‎src/bin/pg_dump/pg_dump.c

Copy file name to clipboardExpand all lines: src/bin/pg_dump/pg_dump.c
+32-5Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8736,13 +8736,37 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
87368736
attrdefs[j].dobj.dump = tbinfo->dobj.dump;
87378737

87388738
/*
8739-
* Defaults on a VIEW must always be dumped as separate ALTER
8740-
* TABLE commands. Defaults on regular tables are dumped as
8741-
* part of the CREATE TABLE if possible, which it won't be if
8742-
* the column is not going to be emitted explicitly.
8739+
* Figure out whether the default/generation expression should
8740+
* be dumped as part of the main CREATE TABLE (or similar)
8741+
* command or as a separate ALTER TABLE (or similar) command.
8742+
* The preference is to put it into the CREATE command, but in
8743+
* some cases that's not possible.
87438744
*/
8744-
if (tbinfo->relkind == RELKIND_VIEW)
8745+
if (tbinfo->attgenerated[adnum - 1])
87458746
{
8747+
/*
8748+
* Column generation expressions cannot be dumped
8749+
* separately, because there is no syntax for it. The
8750+
* !shouldPrintColumn case below will be tempted to set
8751+
* them to separate if they are attached to an inherited
8752+
* column without a local definition, but that would be
8753+
* wrong and unnecessary, because generation expressions
8754+
* are always inherited, so there is no need to set them
8755+
* again in child tables, and there is no syntax for it
8756+
* either. By setting separate to false here we prevent
8757+
* the "default" from being processed as its own dumpable
8758+
* object, and flagInhAttrs() will remove it from the
8759+
* table when it detects that it belongs to an inherited
8760+
* column.
8761+
*/
8762+
attrdefs[j].separate = false;
8763+
}
8764+
else if (tbinfo->relkind == RELKIND_VIEW)
8765+
{
8766+
/*
8767+
* Defaults on a VIEW must always be dumped as separate
8768+
* ALTER TABLE commands.
8769+
*/
87468770
attrdefs[j].separate = true;
87478771
}
87488772
else if (!shouldPrintColumn(dopt, tbinfo, adnum - 1))
@@ -8753,7 +8777,10 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
87538777
else
87548778
{
87558779
attrdefs[j].separate = false;
8780+
}
87568781

8782+
if (!attrdefs[j].separate)
8783+
{
87578784
/*
87588785
* Mark the default as needing to appear before the table,
87598786
* so that any dependencies it has must be emitted before

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

Copy file name to clipboardExpand all lines: src/bin/pg_dump/t/002_pg_dump.pl
+46Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2487,6 +2487,52 @@
24872487
unlike => { exclude_dump_test_schema => 1, },
24882488
},
24892489
2490+
'CREATE TABLE test_table_generated_child1 (without local columns)' => {
2491+
create_order => 4,
2492+
create_sql => 'CREATE TABLE dump_test.test_table_generated_child1 ()
2493+
INHERITS (dump_test.test_table_generated);',
2494+
regexp => qr/^
2495+
\QCREATE TABLE dump_test.test_table_generated_child1 (\E\n
2496+
\)\n
2497+
\QINHERITS (dump_test.test_table_generated);\E\n
2498+
/xms,
2499+
like =>
2500+
{ %full_runs, %dump_test_schema_runs, section_pre_data => 1, },
2501+
unlike => {
2502+
binary_upgrade => 1,
2503+
exclude_dump_test_schema => 1,
2504+
},
2505+
},
2506+
2507+
'ALTER TABLE test_table_generated_child1' => {
2508+
regexp =>
2509+
qr/^\QALTER TABLE ONLY dump_test.test_table_generated_child1 ALTER COLUMN col2 \E/m,
2510+
2511+
# should not get emitted
2512+
like => {},
2513+
},
2514+
2515+
'CREATE TABLE test_table_generated_child2 (with local columns)' => {
2516+
create_order => 4,
2517+
create_sql => 'CREATE TABLE dump_test.test_table_generated_child2 (
2518+
col1 int,
2519+
col2 int
2520+
) INHERITS (dump_test.test_table_generated);',
2521+
regexp => qr/^
2522+
\QCREATE TABLE dump_test.test_table_generated_child2 (\E\n
2523+
\s+\Qcol1 integer,\E\n
2524+
\s+\Qcol2 integer\E\n
2525+
\)\n
2526+
\QINHERITS (dump_test.test_table_generated);\E\n
2527+
/xms,
2528+
like =>
2529+
{ %full_runs, %dump_test_schema_runs, section_pre_data => 1, },
2530+
unlike => {
2531+
binary_upgrade => 1,
2532+
exclude_dump_test_schema => 1,
2533+
},
2534+
},
2535+
24902536
'CREATE TABLE table_with_stats' => {
24912537
create_order => 98,
24922538
create_sql => 'CREATE TABLE dump_test.table_index_stats (

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.