Movatterモバイル変換

[0]ホーム
URL:

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

 Sign up
Appearance settings

Commit2b216da

Browse files
committed
Fix pg_dump for hash partitioning on enum columns.
Hash partitioning on an enum is problematic because the hash codes arederived from the OIDs assigned to the enum values, which will almostcertainly be different after a dump-and-reload than they were before.This means that some rows probably end up in different partitions thanbefore, causing restore to fail because of partition constraintviolations. (pg_upgrade dodges this problem by using hacks to forcethe enum values to keep the same OIDs, but that's not possible nordesirable for pg_dump.)Users can work around that by specifying --load-via-partition-root,but since that's a dump-time not restore-time decision, one mightfind out the need for it far too late. Instead, teach pg_dump toapply that option automatically when dealing with a partitionedtable that has hash-on-enum partitioning.Also deal with a pre-existing issue for --load-via-partition-rootmode: in a parallel restore, we try to TRUNCATE target tables justbefore loading them, in order to enable some backend optimizations.This is bad when using --load-via-partition-root because (a) we'relikely to suffer deadlocks from restore jobs trying to restore rowsinto other partitions than they came from, and (b) if we miss gettinga deadlock we might still lose data due to a TRUNCATE removing rowsfrom some already-completed restore job.The fix for this is conceptually simple: just don't TRUNCATE if we'redealing with a --load-via-partition-root case. The tricky bit is forpg_restore to identify those cases. In dumps using COPY commands wecan inspect each COPY command to see if it targets the nominal targettable or some ancestor. However, in dumps using INSERT commands it'spretty impractical to examine the INSERTs in advance. To provide asolution for that going forward, modify pg_dump to mark TABLE DATAitems that are using --load-via-partition-root with a comment.(This change also responds to a complaint from Robert Haas thatthe dump output for --load-via-partition-root is pretty confusing.)pg_restore checks for the special comment as well as checking theCOPY command if present. This will fail to identify the combinationof --load-via-partition-root and --inserts in pre-existing dump files,but that should be a pretty rare case in the field. If it doeshappen you will probably get a deadlock failure that you can workaround by not using parallel restore, which is the same as beforethis bug fix.Having done this, there seems no remaining reason for the alarmismin the pg_dump man page about combining --load-via-partition-rootwith parallel restore, so remove that warning.Patch by me; thanks to Julien Rouhaud for review. Back-patch tov11 where hash partitioning was introduced.Discussion:https://postgr.es/m/1376149.1675268279@sss.pgh.pa.us
1 parentce29cea commit2b216da

File tree

7 files changed

+287
-52
lines changed

7 files changed

+287
-52
lines changed

‎doc/src/sgml/ref/pg_dump.sgml

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -853,16 +853,6 @@ PostgreSQL documentation
853853
and the two systems have different definitions of the collation used
854854
to sort the partitioning column.
855855
</para>
856-
857-
<para>
858-
It is best not to use parallelism when restoring from an archive made
859-
with this option, because <application>pg_restore</application> will
860-
not know exactly which partition(s) a given archive data item will
861-
load data into. This could result in inefficiency due to lock
862-
conflicts between parallel jobs, or perhaps even restore failures due
863-
to foreign key constraints being set up before all the relevant data
864-
is loaded.
865-
</para>
866856
</listitem>
867857
</varlistentry>
868858

‎doc/src/sgml/ref/pg_dumpall.sgml

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -361,10 +361,6 @@ PostgreSQL documentation
361361
and the two systems have different definitions of the collation used
362362
to sort the partitioning column.
363363
</para>
364-
365-
<!-- Currently, we don't need pg_dump's warning about parallelism here,
366-
since parallel restore from a pg_dumpall script is impossible.
367-
-->
368364
</listitem>
369365
</varlistentry>
370366

‎src/bin/pg_dump/common.c

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,9 @@ getSchemaData(Archive *fout, int *numTablesPtr)
230230
pg_log_info("flagging inherited columns in subtables");
231231
flagInhAttrs(fout->dopt,tblinfo,numTables);
232232

233+
pg_log_info("reading partitioning data");
234+
getPartitioningInfo(fout);
235+
233236
pg_log_info("reading indexes");
234237
getIndexes(fout,tblinfo,numTables);
235238

@@ -285,7 +288,6 @@ static void
285288
flagInhTables(Archive*fout,TableInfo*tblinfo,intnumTables,
286289
InhInfo*inhinfo,intnumInherits)
287290
{
288-
DumpOptions*dopt=fout->dopt;
289291
inti,
290292
j;
291293

@@ -301,18 +303,18 @@ flagInhTables(Archive *fout, TableInfo *tblinfo, int numTables,
301303
continue;
302304

303305
/*
304-
* Normally, we don't bother computing anything for non-target tables,
305-
*but if load-via-partition-root is specified, we gather information
306-
*on every partition in the systemso thatgetRootTableInfo can trace
307-
*from any given toleafpartition all the way up to the root.(We
308-
*don't need tomarkthem as interesting for getTableAttrs, though.)
306+
* Normally, we don't bother computing anything for non-target tables.
307+
*However, we must find the parents of non-root partitioned tables in
308+
*any case,so thatwe can trace from leaf partitions up to the root
309+
*(in case aleafis to be dumped but its parents are not).We need
310+
*notmarksuch parents interesting for getTableAttrs, though.
309311
*/
310312
if (!tblinfo[i].dobj.dump)
311313
{
312314
mark_parents= false;
313315

314-
if (!dopt->load_via_partition_root||
315-
!tblinfo[i].ispartition)
316+
if (!(tblinfo[i].relkind==RELKIND_PARTITIONED_TABLE&&
317+
tblinfo[i].ispartition))
316318
find_parents= false;
317319
}
318320

‎src/bin/pg_dump/pg_backup_archiver.c

Lines changed: 64 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ static RestorePass _tocEntryRestorePass(TocEntry *te);
9191
staticbool_tocEntryIsACL(TocEntry*te);
9292
staticvoid_disableTriggersIfNecessary(ArchiveHandle*AH,TocEntry*te);
9393
staticvoid_enableTriggersIfNecessary(ArchiveHandle*AH,TocEntry*te);
94+
staticboolis_load_via_partition_root(TocEntry*te);
9495
staticvoidbuildTocEntryArrays(ArchiveHandle*AH);
9596
staticvoid_moveBefore(TocEntry*pos,TocEntry*te);
9697
staticint_discoverArchiveFormat(ArchiveHandle*AH);
@@ -884,6 +885,8 @@ restore_toc_entry(ArchiveHandle *AH, TocEntry *te, bool is_parallel)
884885
}
885886
else
886887
{
888+
booluse_truncate;
889+
887890
_disableTriggersIfNecessary(AH,te);
888891

889892
/* Select owner and schema as necessary */
@@ -895,13 +898,24 @@ restore_toc_entry(ArchiveHandle *AH, TocEntry *te, bool is_parallel)
895898

896899
/*
897900
* In parallel restore, if we created the table earlier in
898-
* the run then we wrap the COPY in a transaction and
899-
* precede it with a TRUNCATE. If archiving is not on
900-
* this prevents WAL-logging the COPY. This obtains a
901-
* speedup similar to that from using single_txn mode in
902-
* non-parallel restores.
901+
* this run (so that we know it is empty) and we are not
902+
* restoring a load-via-partition-root data item then we
903+
* wrap the COPY in a transaction and precede it with a
904+
* TRUNCATE. If wal_level is set to minimal this prevents
905+
* WAL-logging the COPY. This obtains a speedup similar
906+
* to that from using single_txn mode in non-parallel
907+
* restores.
908+
*
909+
* We mustn't do this for load-via-partition-root cases
910+
* because some data might get moved across partition
911+
* boundaries, risking deadlock and/or loss of previously
912+
* loaded data. (We assume that all partitions of a
913+
* partitioned table will be treated the same way.)
903914
*/
904-
if (is_parallel&&te->created)
915+
use_truncate=is_parallel&&te->created&&
916+
!is_load_via_partition_root(te);
917+
918+
if (use_truncate)
905919
{
906920
/*
907921
* Parallel restore is always talking directly to a
@@ -939,7 +953,7 @@ restore_toc_entry(ArchiveHandle *AH, TocEntry *te, bool is_parallel)
939953
AH->outputKind=OUTPUT_SQLCMDS;
940954

941955
/* close out the transaction started above */
942-
if (is_parallel&&te->created)
956+
if (use_truncate)
943957
CommitTransaction(&AH->public);
944958

945959
_enableTriggersIfNecessary(AH,te);
@@ -1031,6 +1045,43 @@ _enableTriggersIfNecessary(ArchiveHandle *AH, TocEntry *te)
10311045
fmtQualifiedId(te->namespace,te->tag));
10321046
}
10331047

1048+
/*
1049+
* Detect whether a TABLE DATA TOC item is performing "load via partition
1050+
* root", that is the target table is an ancestor partition rather than the
1051+
* table the TOC item is nominally for.
1052+
*
1053+
* In newer archive files this can be detected by checking for a special
1054+
* comment placed in te->defn. In older files we have to fall back to seeing
1055+
* if the COPY statement targets the named table or some other one. This
1056+
* will not work for data dumped as INSERT commands, so we could give a false
1057+
* negative in that case; fortunately, that's a rarely-used option.
1058+
*/
1059+
staticbool
1060+
is_load_via_partition_root(TocEntry*te)
1061+
{
1062+
if (te->defn&&
1063+
strncmp(te->defn,"-- load via partition root ",27)==0)
1064+
return true;
1065+
if (te->copyStmt&&*te->copyStmt)
1066+
{
1067+
PQExpBuffercopyStmt=createPQExpBuffer();
1068+
boolresult;
1069+
1070+
/*
1071+
* Build the initial part of the COPY as it would appear if the
1072+
* nominal target table is the actual target. If we see anything
1073+
* else, it must be a load-via-partition-root case.
1074+
*/
1075+
appendPQExpBuffer(copyStmt,"COPY %s ",
1076+
fmtQualifiedId(te->namespace,te->tag));
1077+
result=strncmp(te->copyStmt,copyStmt->data,copyStmt->len)!=0;
1078+
destroyPQExpBuffer(copyStmt);
1079+
returnresult;
1080+
}
1081+
/* Assume it's not load-via-partition-root */
1082+
return false;
1083+
}
1084+
10341085
/*
10351086
* This is a routine that is part of the dumper interface, hence the 'Archive*' parameter.
10361087
*/
@@ -2974,8 +3025,12 @@ _tocEntryRequired(TocEntry *te, teSection curSection, ArchiveHandle *AH)
29743025
res=res& ~REQ_DATA;
29753026
}
29763027

2977-
/* If there's no definition command, there's no schema component */
2978-
if (!te->defn|| !te->defn[0])
3028+
/*
3029+
* If there's no definition command, there's no schema component. Treat
3030+
* "load via partition root" comments as not schema.
3031+
*/
3032+
if (!te->defn|| !te->defn[0]||
3033+
strncmp(te->defn,"-- load via partition root ",27)==0)
29793034
res=res& ~REQ_SCHEMA;
29803035

29813036
/*

‎src/bin/pg_dump/pg_dump.c

Lines changed: 130 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,7 @@ static void appendReloptionsArrayAH(PQExpBuffer buffer, const char *reloptions,
317317
static char *get_synchronized_snapshot(Archive *fout);
318318
static void setupDumpWorker(Archive *AHX);
319319
static TableInfo *getRootTableInfo(const TableInfo *tbinfo);
320+
static bool forcePartitionRootLoad(const TableInfo *tbinfo);
320321

321322

322323
int
@@ -2192,11 +2193,13 @@ dumpTableData_insert(Archive *fout, const void *dcontext)
21922193
insertStmt = createPQExpBuffer();
21932194

21942195
/*
2195-
* When load-via-partition-root is set, get the root table name
2196-
* for the partition table, so that we can reload data through the
2197-
* root table.
2196+
* When load-via-partition-root is set or forced, get the root
2197+
*table namefor the partition table, so that we can reload data
2198+
*through theroot table.
21982199
*/
2199-
if (dopt->load_via_partition_root && tbinfo->ispartition)
2200+
if (tbinfo->ispartition &&
2201+
(dopt->load_via_partition_root ||
2202+
forcePartitionRootLoad(tbinfo)))
22002203
targettab = getRootTableInfo(tbinfo);
22012204
else
22022205
targettab = tbinfo;
@@ -2394,6 +2397,35 @@ getRootTableInfo(const TableInfo *tbinfo)
23942397
return parentTbinfo;
23952398
}
23962399

2400+
/*
2401+
* forcePartitionRootLoad
2402+
* Check if we must force load_via_partition_root for this partition.
2403+
*
2404+
* This is required if any level of ancestral partitioned table has an
2405+
* unsafe partitioning scheme.
2406+
*/
2407+
static bool
2408+
forcePartitionRootLoad(const TableInfo *tbinfo)
2409+
{
2410+
TableInfo *parentTbinfo;
2411+
2412+
Assert(tbinfo->ispartition);
2413+
Assert(tbinfo->numParents == 1);
2414+
2415+
parentTbinfo = tbinfo->parents[0];
2416+
if (parentTbinfo->unsafe_partitions)
2417+
return true;
2418+
while (parentTbinfo->ispartition)
2419+
{
2420+
Assert(parentTbinfo->numParents == 1);
2421+
parentTbinfo = parentTbinfo->parents[0];
2422+
if (parentTbinfo->unsafe_partitions)
2423+
return true;
2424+
}
2425+
2426+
return false;
2427+
}
2428+
23972429
/*
23982430
* dumpTableData -
23992431
* dump the contents of a single table
@@ -2408,34 +2440,40 @@ dumpTableData(Archive *fout, const TableDataInfo *tdinfo)
24082440
PQExpBuffer copyBuf = createPQExpBuffer();
24092441
PQExpBuffer clistBuf = createPQExpBuffer();
24102442
DataDumperPtr dumpFn;
2443+
char *tdDefn = NULL;
24112444
char *copyStmt;
24122445
const char *copyFrom;
24132446

24142447
/* We had better have loaded per-column details about this table */
24152448
Assert(tbinfo->interesting);
24162449

2450+
/*
2451+
* When load-via-partition-root is set or forced, get the root table name
2452+
* for the partition table, so that we can reload data through the root
2453+
* table. Then construct a comment to be inserted into the TOC entry's
2454+
* defn field, so that such cases can be identified reliably.
2455+
*/
2456+
if (tbinfo->ispartition &&
2457+
(dopt->load_via_partition_root ||
2458+
forcePartitionRootLoad(tbinfo)))
2459+
{
2460+
TableInfo *parentTbinfo;
2461+
2462+
parentTbinfo = getRootTableInfo(tbinfo);
2463+
copyFrom = fmtQualifiedDumpable(parentTbinfo);
2464+
printfPQExpBuffer(copyBuf, "-- load via partition root %s",
2465+
copyFrom);
2466+
tdDefn = pg_strdup(copyBuf->data);
2467+
}
2468+
else
2469+
copyFrom = fmtQualifiedDumpable(tbinfo);
2470+
24172471
if (dopt->dump_inserts == 0)
24182472
{
24192473
/* Dump/restore using COPY */
24202474
dumpFn = dumpTableData_copy;
2421-
2422-
/*
2423-
* When load-via-partition-root is set, get the root table name for
2424-
* the partition table, so that we can reload data through the root
2425-
* table.
2426-
*/
2427-
if (dopt->load_via_partition_root && tbinfo->ispartition)
2428-
{
2429-
TableInfo *parentTbinfo;
2430-
2431-
parentTbinfo = getRootTableInfo(tbinfo);
2432-
copyFrom = fmtQualifiedDumpable(parentTbinfo);
2433-
}
2434-
else
2435-
copyFrom = fmtQualifiedDumpable(tbinfo);
2436-
24372475
/* must use 2 steps here 'cause fmtId is nonreentrant */
2438-
appendPQExpBuffer(copyBuf, "COPY %s ",
2476+
printfPQExpBuffer(copyBuf, "COPY %s ",
24392477
copyFrom);
24402478
appendPQExpBuffer(copyBuf, "%s FROM stdin;\n",
24412479
fmtCopyColumnList(tbinfo, clistBuf));
@@ -2463,6 +2501,7 @@ dumpTableData(Archive *fout, const TableDataInfo *tdinfo)
24632501
.owner = tbinfo->rolname,
24642502
.description = "TABLE DATA",
24652503
.section = SECTION_DATA,
2504+
.createStmt = tdDefn,
24662505
.copyStmt = copyStmt,
24672506
.deps = &(tbinfo->dobj.dumpId),
24682507
.nDeps = 1,
@@ -6654,6 +6693,76 @@ getInherits(Archive *fout, int *numInherits)
66546693
return inhinfo;
66556694
}
66566695

6696+
/*
6697+
* getPartitioningInfo
6698+
* get information about partitioning
6699+
*
6700+
* For the most part, we only collect partitioning info about tables we
6701+
* intend to dump. However, this function has to consider all partitioned
6702+
* tables in the database, because we need to know about parents of partitions
6703+
* we are going to dump even if the parents themselves won't be dumped.
6704+
*
6705+
* Specifically, what we need to know is whether each partitioned table
6706+
* has an "unsafe" partitioning scheme that requires us to force
6707+
* load-via-partition-root mode for its children. Currently the only case
6708+
* for which we force that is hash partitioning on enum columns, since the
6709+
* hash codes depend on enum value OIDs which won't be replicated across
6710+
* dump-and-reload. There are other cases in which load-via-partition-root
6711+
* might be necessary, but we expect users to cope with them.
6712+
*/
6713+
void
6714+
getPartitioningInfo(Archive *fout)
6715+
{
6716+
PQExpBuffer query;
6717+
PGresult *res;
6718+
intntups;
6719+
6720+
/* hash partitioning didn't exist before v11 */
6721+
if (fout->remoteVersion < 110000)
6722+
return;
6723+
/* needn't bother if schema-only dump */
6724+
if (fout->dopt->schemaOnly)
6725+
return;
6726+
6727+
query = createPQExpBuffer();
6728+
6729+
/*
6730+
* Unsafe partitioning schemes are exactly those for which hash enum_ops
6731+
* appears among the partition opclasses. We needn't check partstrat.
6732+
*
6733+
* Note that this query may well retrieve info about tables we aren't
6734+
* going to dump and hence have no lock on. That's okay since we need not
6735+
* invoke any unsafe server-side functions.
6736+
*/
6737+
appendPQExpBufferStr(query,
6738+
"SELECT partrelid FROM pg_partitioned_table WHERE\n"
6739+
"(SELECT c.oid FROM pg_opclass c JOIN pg_am a "
6740+
"ON c.opcmethod = a.oid\n"
6741+
"WHERE opcname = 'enum_ops' "
6742+
"AND opcnamespace = 'pg_catalog'::regnamespace "
6743+
"AND amname = 'hash') = ANY(partclass)");
6744+
6745+
res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
6746+
6747+
ntups = PQntuples(res);
6748+
6749+
for (int i = 0; i < ntups; i++)
6750+
{
6751+
Oidtabrelid = atooid(PQgetvalue(res, i, 0));
6752+
TableInfo *tbinfo;
6753+
6754+
tbinfo = findTableByOid(tabrelid);
6755+
if (tbinfo == NULL)
6756+
pg_fatal("failed sanity check, table OID %u appearing in pg_partitioned_table not found",
6757+
tabrelid);
6758+
tbinfo->unsafe_partitions = true;
6759+
}
6760+
6761+
PQclear(res);
6762+
6763+
destroyPQExpBuffer(query);
6764+
}
6765+
66576766
/*
66586767
* getIndexes
66596768
* get information about every index on a dumpable table

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp