8000 Make pg_dump emit ATTACH PARTITION instead of PARTITION OF (reprise) · postgres/postgres@33a5313 · GitHub
[go: up one dir, main page]
Skip to content

Commit 33a5313

Browse files
alvherrealvherre
committed
Make pg_dump emit ATTACH PARTITION instead of PARTITION OF (reprise)
Using PARTITION OF can 8000 result in column ordering being changed from the database being dumped, if the partition uses a column layout different from the parent's. It's not pg_dump's job to editorialize on table definitions, so this is not acceptable; back-patch all the way back to pg10, where partitioned tables where introduced. This change also ensures that partitions end up in the correct tablespace, if different from the parent's; this is an oversight in ca41030 (in pg12 only). Partitioned indexes (in pg11) don't have this problem, because they're already created as independent indexes and attached to their parents afterwards. This change also has the advantage that the partition is restorable from the dump (as a standalone table) even if its parent table isn't restored. The original commits (3b23552 in branch master) failed to cover subsidiary column elements correctly, such as NOT NULL constraint and CHECK constraints, as reported by Rushabh Lathia (initially as a failure to restore serial columns). They were reverted. This recapitulation commit fixes those problems. Add some pg_dump tests to verify these things more exhaustively, including constraints with legacy-inheritance tables, which were not tested originally. In branches 10 and 11, add a local constraint to the pg_dump test partition that was added by commit 2d7eeb1 to master. Author: Álvaro Herrera, David Rowley Reviewed-by: Álvaro Herrera Discussion: https://postgr.es/m/CAKJS1f_1c260nOt_vBJ067AZ3JXptXVRohDVMLEBmudX1YEx-A@mail.gmail.com Discussion: https://postgr.es/m/20190423185007.GA27954@alvherre.pgsql Discussion: https://postgr.es/m/CAGPqQf0iQV=PPOv2Btog9J9AwOQp6HmuVd6SbGTR_v3Zp2XT1w@mail.gmail.com
1 parent b6987a8 commit 33a5313

File tree

2 files changed

+138
-92
lines changed

2 files changed

+138
-92
lines changed

‎src/bin/pg_dump/pg_dump.c

Lines changed: 78 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -8631,9 +8631,12 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
86318631
* Normally this is always true, but it's false for dropped columns, as well
86328632
* as those that were inherited without any local definition. (If we print
86338633
* such a column it will mistakenly get pg_attribute.attislocal set to true.)
8634-
* However, in binary_upgrade mode, we must print all such columns anyway and
8635-
* fix the attislocal/attisdropped state later, so as to keep control of the
8636-
* physical column order.
8634+
* For partitions, it's always true, because we want the partitions to be
8635+
* created independently and ATTACH PARTITION used afterwards.
8636+
*
8637+
* In binary_upgrade mode, we must print all columns and fix the attislocal/
8638+
* attisdropped state later, so as to keep control of the physical column
8639+
* order.
86378640
*
86388641
* This function exists because there are scattered nonobvious places that
86398642
* must be kept in sync with this decision.
@@ -8643,7 +8646,9 @@ shouldPrintColumn(DumpOptions *dopt, TableInfo *tbinfo, int colno)
86438646
{
86448647
if (dopt->binary_upgrade)
86458648
return true;
8646-
return (tbinfo->attislocal[colno] && !tbinfo->attisdropped[colno]);
8649+
if (tbinfo->attisdropped[colno])
8650+
return false;
8651+
return (tbinfo->attislocal[colno] || tbinfo->ispartition);
86478652
}
86488653

86498654

@@ -15594,27 +15599,6 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
1559415599
if (tbinfo->reloftype && !dopt->binary_upgrade)
1559515600
appendPQExpBuffer(q, " OF %s", tbinfo->reloftype);
1559615601

15597-
/*
15598-
* If the table is a partition, dump it as such; except in the case of
15599-
* a binary upgrade, we dump the table normally and attach it to the
15600-
* parent afterward.
15601-
*/
15602-
if (tbinfo->ispartition && !dopt->binary_upgrade)
15603-
{
15604-
TableInfo *parentRel = tbinfo->parents[0];
15605-
15606-
/*
15607-
* With partitions, unlike inheritance, there can only be one
15608-
* parent.
15609-
*/
15610-
if (tbinfo->numParents != 1)
15611-
fatal("invalid number of parents %d for table \"%s\"",
15612-
tbinfo->numParents, tbinfo->dobj.name);
15613-
15614-
appendPQExpBuffer(q, " PARTITION OF %s",
15615-
fmtQualifiedDumpable(parentRel));
15616-
}
15617-
1561815602
if (tbinfo->relkind != RELKIND_MATVIEW)
1561915603
{
1562015604
/* Dump the attributes */
@@ -15629,26 +15613,30 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
1562915613
*/
1563015614
if (shouldPrintColumn(dopt, tbinfo, j))
1563115615
{
15616+
bool print_default;
15617+
bool print_notnull;
15618+
1563215619
/*
1563315620
* Default value --- suppress if to be printed separately.
1563415621
*/
15635-
bool has_default = (tbinfo->attrdefs[j] != NULL &&
15636-
!tbinfo->attrdefs[j]->separate);
15622+
print_default = (tbinfo->attrdefs[j] != NULL &&
15623+
!tbinfo->attrdefs[j]->separate);
1563715624

1563815625
/*
1563915626
* Not Null constraint --- suppress if inherited, except
15640-
* in binary-upgrade case where that won't work.
15627+
* if partition, or in binary-upgrade case where that
15628+
* won't work.
1564115629
*/
15642-
bool has_notnull = (tbinfo->notnull[j] &&
15643-
(!tbinfo->inhNotNull[j] ||
15644-
dopt->binary_upgrade));
15630+
print_notnull = (tbinfo->notnull[j] &&
15631+
(!tbinfo->inhNotNull[j] ||
15632+
tbinfo->ispartition || dopt->binary_upgrade));
1564515633

1564615634
/*
15647-
* Skip column if fully defined by reloftype or the
15648-
* partition parent.
15635+
* Skip column if fully defined by reloftype, except in
15636+
* binary upgrade
1564915637
*/
15650-
if ((tbinfo->reloftype || tbinfo->ispartition) &&
15651-
!has_default && !has_notnull && !dopt->binary_upgrade)
15638+
if (tbinfo->reloftype && !print_default && !print_notnull &&
15639+
!dopt->binary_upgrade)
1565215640
continue;
1565315641

1565415642
/* Format properly if not first attr */
@@ -15671,26 +15659,22 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
1567115659
* clean things up later.
1567215660
*/
1567315661
appendPQExpBufferStr(q, " INTEGER /* dummy */");
15674-
/* Skip all the rest, too */
15662+
/* and skip to the next column */
1567515663
continue;
1567615664
}
1567715665

1567815666
/*
15679-
* Attribute type
15680-
*
15681-
* In binary-upgrade mode, we always include the type. If
15682-
* we aren't in binary-upgrade mode, then we skip the type
15683-
* when creating a typed table ('OF type_name') or a
15684-
* partition ('PARTITION OF'), since the type comes from
15685-
* the parent/partitioned table.
15667+
* Attribute type; print it except when creating a typed
15668+
* table ('OF type_name'), but in binary-upgrade mode,
15669+
* print it in that case too.
1568615670
*/
15687-
if (dopt->binary_upgrade || (!tbinfo->reloftype && !tbinfo->ispartition))
15671+
if (dopt->binary_upgrade || !tbinfo->reloftype)
1568815672
{
1568915673
appendPQExpBuffer(q, " %s",
1569015674
tbinfo->atttypnames[j]);
1569115675
}
1569215676

15693-
if (has_default)
15677+
if (print_default)
1569415678
{
1569515679
if (tbinfo->attgenerated[j] == ATTRIBUTE_GENERATED_STORED)
1569615680
appendPQExpBuffer(q, " GENERATED ALWAYS AS (%s) STORED",
@@ -15701,7 +15685,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
1570115685
}
1570215686

1570315687

15704-
if (has_notnull)
15688+
if (print_notnull)
1570515689
appendPQExpBufferStr(q, " NOT NULL");
1570615690

1570715691
/* Add collation if not default for the type */
@@ -15719,12 +15703,18 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
1571915703

1572015704
/*
1572115705
* Add non-inherited CHECK constraints, if any.
15706+
*
15707+
* For partitions, we need to include check constraints even if
15708+
* they're not defined locally, because the ALTER TABLE ATTACH
15709+
* PARTITION that we'll emit later expects the constraint to be
15710+
* there. (No need to fix conislocal: ATTACH PARTITION does that)
1572215711
*/
1572315712
for (j = 0; j < tbinfo->ncheck; j++)
1572415713
{
1572515714
ConstraintInfo *constr = &(tbinfo->checkexprs[j]);
1572615715

15727-
if (constr->separate || !constr->conislocal)
15716+
if (constr->separate ||
15717+
(!constr->conislocal && !tbinfo->ispartition))
1572815718
continue;
1572915719

1573015720
if (actual_atts == 0)
@@ -15741,25 +15731,20 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
1574115731

1574215732
if (actual_atts)
1574315733
appendPQExpBufferStr(q, "\n)");
15744-
else if (!((tbinfo->reloftype || tbinfo->ispartition) &&
15745-
!dopt->binary_upgrade))
15734+
else if (!(tbinfo->reloftype && !dopt->binary_upgrade))
1574615735
{
1574715736
/*
15748-
* We must have a parenthesized attribute list, even though
15749-
* empty, when not using the OF TYPE or PARTITION OF syntax.
15737+
* No attributes? we must have a parenthesized attribute list,
15738+
* even though empty, when not using the OF TYPE syntax.
1575015739
*/
1575115740
appendPQExpBufferStr(q, " (\n)");
1575215741
}
1575315742

15754-
if (tbinfo->ispartition && !dopt->binary_upgrade)
15755-
{
15756-
appendPQExpBufferChar(q, '\n');
15757-
appendPQExpBufferStr(q, tbinfo->partbound);
15758-
}
15759-
15760-
/* Emit the INHERITS clause, except if this is a partition. */
15761-
if (numParents > 0 &&
15762-
!tbinfo->ispartition &&
15743+
/*
15744+
* Emit the INHERITS clause (not for partitions), except in
15745+
* binary-upgrade mode.
15746+
*/
15747+
if (numParents > 0 && !tbinfo->ispartition &&
1576315748
!dopt->binary_upgrade)
1576415749
{
1576515750
appendPQExpBufferStr(q, "\nINHERITS (");
@@ -15910,11 +15895,17 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
1591015895
}
1591115896
}
1591215897

15898+
/*
15899+
* Add inherited CHECK constraints, if any.
15900+
*
15901+
* For partitions, they were already dumped, and conislocal
15902+
* doesn't need fixing.
15903+
*/
1591315904
for (k = 0; k < tbinfo->ncheck; k++)
1591415905
{
1591515906
ConstraintInfo *constr = &(tbinfo->checkexprs[k]);
1591615907

15917-
if (constr->separate || constr->conislocal)
15908+
if (constr->separate || constr->conislocal || tbinfo->ispartition)
1591815909
continue;
1591915910

1592015911
appendPQExpBufferStr(q, "\n-- For binary upgrade, set up inherited constraint.\n");
@@ -15932,30 +15923,16 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
1593215923
appendPQExpBufferStr(q, "::pg_catalog.regclass;\n");
1593315924
}
1593415925

15935-
if (numParents > 0)
15926+
if (numParents > 0 && !tbinfo->ispartition)
1593615927
{
15937-
appendPQExpBufferStr(q, "\n-- For binary upgrade, set up inheritance and partitioning this way.\n");
15928+
appendPQExpBufferStr(q, "\n-- For binary upgrade, set up inheritance this way.\n");
1593815929
for (k = 0; k < numParents; k++)
1593915930
{
1594015931
TableInfo *parentRel = parents[k];
1594115932

15942-
/* In the partitioning case, we alter the parent */
15943-
if (tbinfo->ispartition)
15944-
appendPQExpBuffer(q,
15945-
"ALTER TABLE ONLY %s ATTACH PARTITION ",
15946-
fmtQualifiedDumpable(parentRel));
15947-
else
15948-
appendPQExpBuffer(q, "ALTER TABLE ONLY %s INHERIT ",
15949-
qualrelname);
15950-
15951-
/* Partition needs specifying the bounds */
15952-
if (tbinfo->ispartition)
15953-
appendPQExpBuffer(q, "%s %s;\n",
15954-
qualrelname,
15955-
tbinfo->partbound);
15956-
else
15957-
appendPQExpBuffer(q, "%s;\n",
15958-
fmtQualifiedDumpable(parentRel));
15933+
appendPQExpBuffer(q, "ALTER TABLE ONLY %s INHERIT %s;\n",
15934+
qualrelname,
15935+
fmtQualifiedDumpable(parentRel));
1595915936
}
1596015937
}
1596115938

@@ -15968,6 +15945,27 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
1596815945
}
1596915946
}
1597015947

15948+
/*
15949+
* For partitioned tables, emit the ATTACH PARTITION clause. Note
15950+
* that we always want to create partitions this way instead of using
15951+
* CREATE TABLE .. PARTITION OF, mainly to preserve a possible column
15952+
* layout discrepancy with the parent, but also to ensure it gets the
15953+
F438 * correct tablespace setting if it differs from the parent's.
15954+
*/
15955+
if (tbinfo->ispartition)
15956+
{
15957+
/* With partitions there can only be one parent */
15958+
if (tbinfo->numParents != 1)
15959+
fatal("invalid number of parents %d for table \"%s\"",
15960+
tbinfo->numParents, tbinfo->dobj.name);
15961+
15962+
/* Perform ALTER TABLE on the parent */
15963+
appendPQExpBuffer(q,
15964+
"ALTER TABLE ONLY %s ATTACH PARTITION %s %s;\n",
15965+
fmtQualifiedDumpable(parents[0]),
15966+
qualrelname, tbinfo->partbound);
15967+
}
15968+
1597115969
/*
1597215970
* In binary_upgrade mode, arrange to restore the old relfrozenxid and
1597315971
* relminmxid of all vacuumable relations. (While vacuum.c processes

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

Lines changed: 60 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -733,7 +733,12 @@
733733
\QALTER TABLE ONLY dump_test.measurement ATTACH PARTITION dump_test_second_schema.measurement_y2006m2 \E
734734
\QFOR VALUES FROM ('2006-02-01') TO ('2006-03-01');\E\n
735735
/xm,
736-
like => { binary_upgrade => 1, },
736+
like => {
737+
%full_runs,
738+
role => 1,
739+
section_pre_data => 1,
740+
binary_upgrade => 1,
741+
},
737742
},
738743

739744
'ALTER TABLE test_table CLUSTER ON test_table_pkey' => {
@@ -2283,9 +2288,9 @@
22832288
'CREATE TABLE measurement PARTITIONED BY' => {
22842289
create_order => 90,
22852290
create_sql => 'CREATE TABLE dump_test.measurement (
2286-
city_id int not null,
2291+
city_id serial not null,
22872292
logdate date not null,
2288-
peaktemp int,
2293+
peaktemp int CHECK (peaktemp >= -460),
22892294
unitsales int
22902295
) PARTITION BY RANGE (logdate);',
22912296
regexp => qr/^
@@ -2295,7 +2300,8 @@
22952300
\s+\Qcity_id integer NOT NULL,\E\n
22962301
\s+\Qlogdate date NOT NULL,\E\n
22972302
\s+\Qpeaktemp integer,\E\n
2298-
\s+\Qunitsales integer\E\n
2303+
\s+\Qunitsales integer,\E\n
2304+
\s+\QCONSTRAINT measurement_peaktemp_check CHECK ((peaktemp >= '-460'::integer))\E\n
22992305
\)\n
23002306
\QPARTITION BY RANGE (logdate);\E\n
23012307
/xm,
@@ -2307,7 +2313,7 @@
23072313
},
23082314
},
23092315
2310-
'CREATE TABLE measurement_y2006m2 PARTITION OF' => {
2316+
'Partition measurement_y2006m2 creation' => {
23112317
create_order => 91,
23122318
create_sql =>
23132319
'CREATE TABLE dump_test_second_schema.measurement_y2006m2
@@ -2316,19 +2322,21 @@
23162322
)
23172323
FOR VALUES FROM (\'2006-02-01\') TO (\'2006-03-01\');',
23182324
regexp => qr/^
2319-
\Q-- Name: measurement_y2006m2;\E.*\n
2320-
\Q--\E\n\n
2321-
\QCREATE TABLE dump_test_second_schema.measurement_y2006m2 PARTITION OF dump_test.measurement (\E\n
2325+
\QCREATE TABLE dump_test_second_schema.measurement_y2006m2 (\E\n
2326+
\s+\Qcity_id integer DEFAULT nextval('dump_test.measurement_city_id_seq'::regclass) NOT NULL,\E\n
2327+
\s+\Qlogdate date NOT NULL,\E\n
2328+
\s+\Qpeaktemp integer,\E\n
2329+
\s+\Qunitsales integer DEFAULT 0,\E\n
2330+
\s+\QCONSTRAINT measurement_peaktemp_check CHECK ((peaktemp >= '-460'::integer)),\E\n
23222331
\s+\QCONSTRAINT measurement_y2006m2_unitsales_check CHECK ((unitsales >= 0))\E\n
2323-
\)\n
2324-
\QFOR VALUES FROM ('2006-02-01') TO ('2006-03-01');\E\n
2332+
\);\n
23252333
/xm,
23262334
like => {
23272335
%full_runs,
2328-
role => 1,
23292336
section_pre_data => 1,
2337+
role => 1,
2338+
binary_upgrade => 1,
23302339
},
2331-
unlike => { binary_upgrade => 1, },
23322340
},
23332341
23342342
'CREATE TABLE test_fourth_table_zero_col' => {
@@ -2432,6 +2440,46 @@
24322440
unlike => { exclude_dump_test_schema => 1, },
24332441
},
24342442
2443+
'CREATE TABLE test_inheritance_parent' => {
2444+
create_order => 90,
2445+
create_sql => 'CREATE TABLE dump_test.test_inheritance_parent (
2446+
col1 int NOT NULL,
2447+
col2 int CHECK (col2 >= 42)
2448+
);',
2449+
regexp => qr/^
2450+
\QCREATE TABLE dump_test.test_inheritance_parent (\E\n
2451+
\s+\Qcol1 integer NOT NULL,\E\n
2452+
\s+\Qcol2 integer,\E\n
2453+
\s+\QCONSTRAINT test_inheritance_parent_col2_check CHECK ((col2 >= 42))\E\n
2454+
\Q);\E\n
2455+
/xm,
2456+
like =>
2457+
{ %full_runs, %dump_test_schema_runs, section_pre_data => 1, },
2458+
unlike => { exclude_dump_test_schema => 1, },
2459+
},
2460+
2461+
'CREATE TABLE test_inheritance_child' => {
2462+
create_order => 91,
2463+
create_sql => 'CREATE TABLE dump_test.test_inheritance_child (
2464+
col1 int NOT NULL,
2465+
CONSTRAINT test_inheritance_child CHECK (col2 >= 142857)
2466+
) INHERITS (dump_test.test_inheritance_parent);',
2467+
regexp => qr/^
2468+
\QCREATE TABLE dump_test.test_inheritance_child (\E\n
2469+
\s+\Qcol1 integer,\E\n
2470+
\s+\QCONSTRAINT test_inheritance_child CHECK ((col2 >= 142857))\E\n
2471+
\)\n
2472+
\QINHERITS (dump_test.test_inheritance_parent);\E\n
2473+
/xm,
2474+
like => {
2475+
%full_runs, %dump_test_schema_runs, section_pre_data => 1,
2476+
},
2477+
unlike => {
2478+
binary_upgrade => 1,
2479+
exclude_dump_test_schema => 1,
2480+
},
2481+
},
2482+
24352483
'CREATE STATISTICS extended_stats_no_options' => {
24362484
create_order => 97,
24372485
create_sql => 'CREATE STATISTICS dump_test.test_ext_stats_no_options

0 commit comments

Comments
 (0)
0