Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 064709f

Browse files
committed
Simplify and speed up pg_dump's creation of parent-table links.
Instead of trying to optimize this by skipping creation of the links for tables we don't plan to dump, just create them all in bulk with a single scan over the pg_inherits data. The previous approach was more or less O(N^2) in the number of pg_inherits entries, not to mention being way too complicated. Also, don't create useless TableAttachInfo objects. It's silly to create a TableAttachInfo object that we're not going to dump, when we know perfectly well at creation time that it won't be dumped. Patch by me; thanks to Julien Rouhaud for review. Discussion: https://postgr.es/m/1376149.1675268279@sss.pgh.pa.us
1 parent bc8cd50 commit 064709f

File tree

3 files changed

+56
-80
lines changed

3 files changed

+56
-80
lines changed

src/bin/pg_dump/common.c

+53-75
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,6 @@ static void flagInhTables(Archive *fout, TableInfo *tblinfo, int numTables,
8383
InhInfo *inhinfo, int numInherits);
8484
static void flagInhIndexes(Archive *fout, TableInfo *tblinfo, int numTables);
8585
static void flagInhAttrs(DumpOptions *dopt, TableInfo *tblinfo, int numTables);
86-
static void findParentsByOid(TableInfo *self,
87-
InhInfo *inhinfo, int numInherits);
8886
static int strInArray(const char *pattern, char **arr, int arr_size);
8987
static IndxInfo *findIndexByOid(Oid oid);
9088

@@ -288,45 +286,70 @@ static void
288286
flagInhTables(Archive *fout, TableInfo *tblinfo, int numTables,
289287
InhInfo *inhinfo, int numInherits)
290288
{
289+
TableInfo *child = NULL;
290+
TableInfo *parent = NULL;
291291
int i,
292292
j;
293293

294-
for (i = 0; i < numTables; i++)
294+
/*
295+
* Set up links from child tables to their parents.
296+
*
297+
* We used to attempt to skip this work for tables that are not to be
298+
* dumped; but the optimizable cases are rare in practice, and setting up
299+
* these links in bulk is cheaper than the old way. (Note in particular
300+
* that it's very rare for a child to have more than one parent.)
301+
*/
302+
for (i = 0; i < numInherits; i++)
295303
{
296-
bool find_parents = true;
297-
bool mark_parents = true;
298-
299-
/* Some kinds never have parents */
300-
if (tblinfo[i].relkind == RELKIND_SEQUENCE ||
301-
tblinfo[i].relkind == RELKIND_VIEW ||
302-
tblinfo[i].relkind == RELKIND_MATVIEW)
303-
continue;
304-
305304
/*
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 that we can trace from leaf partitions up to the root
309-
* (in case a leaf is to be dumped but its parents are not). We need
310-
* not mark such parents interesting for getTableAttrs, though.
305+
* Skip a hashtable lookup if it's same table as last time. This is
306+
* unlikely for the child, but less so for the parent. (Maybe we
307+
* should ask the backend for a sorted array to make it more likely?
308+
* Not clear the sorting effort would be repaid, though.)
311309
*/
312-
if (!tblinfo[i].dobj.dump)
310+
if (child == NULL ||
311+
child->dobj.catId.oid != inhinfo[i].inhrelid)
313312
{
314-
mark_parents = false;
313+
child = findTableByOid(inhinfo[i].inhrelid);
315314

316-
if (!(tblinfo[i].relkind == RELKIND_PARTITIONED_TABLE &&
317-
tblinfo[i].ispartition))
318-
find_parents = false;
315+
/*
316+
* If we find no TableInfo, assume the pg_inherits entry is for a
317+
* partitioned index, which we don't need to track.
318+
*/
319+
if (child == NULL)
320+
continue;
319321
}
322+
if (parent == NULL ||
323+
parent->dobj.catId.oid != inhinfo[i].inhparent)
324+
{
325+
parent = findTableByOid(inhinfo[i].inhparent);
326+
if (parent == NULL)
327+
pg_fatal("failed sanity check, parent OID %u of table \"%s\" (OID %u) not found",
328+
inhinfo[i].inhparent,
329+
child->dobj.name,
330+
child->dobj.catId.oid);
331+
}
332+
/* Add this parent to the child's list of parents. */
333+
if (child->numParents > 0)
334+
child->parents = pg_realloc_array(child->parents,
335+
TableInfo *,
336+
child->numParents + 1);
337+
else
338+
child->parents = pg_malloc_array(TableInfo *, 1);
339+
child->parents[child->numParents++] = parent;
340+
}
320341

321-
/* If needed, find all the immediate parent tables. */
322-
if (find_parents)
323-
findParentsByOid(&tblinfo[i], inhinfo, numInherits);
324-
342+
/*
343+
* Now consider all child tables and mark parents interesting as needed.
344+
*/
345+
for (i = 0; i < numTables; i++)
346+
{
325347
/*
326348
* If needed, mark the parents as interesting for getTableAttrs and
327-
* getIndexes.
349+
* getIndexes. We only need this for direct parents of dumpable
350+
* tables.
328351
*/
329-
if (mark_parents)
352+
if (tblinfo[i].dobj.dump)
330353
{
331354
int numParents = tblinfo[i].numParents;
332355
TableInfo **parents = tblinfo[i].parents;
@@ -336,7 +359,8 @@ flagInhTables(Archive *fout, TableInfo *tblinfo, int numTables,
336359
}
337360

338361
/* Create TableAttachInfo object if needed */
339-
if (tblinfo[i].dobj.dump && tblinfo[i].ispartition)
362+
if ((tblinfo[i].dobj.dump & DUMP_COMPONENT_DEFINITION) &&
363+
tblinfo[i].ispartition)
340364
{
341365
TableAttachInfo *attachinfo;
342366

@@ -995,52 +1019,6 @@ findOwningExtension(CatalogId catalogId)
9951019
}
9961020

9971021

998-
/*
999-
* findParentsByOid
1000-
* find a table's parents in tblinfo[]
1001-
*/
1002-
static void
1003-
findParentsByOid(TableInfo *self,
1004-
InhInfo *inhinfo, int numInherits)
1005-
{
1006-
Oid oid = self->dobj.catId.oid;
1007-
int i,
1008-
j;
1009-
int numParents;
1010-
1011-
numParents = 0;
1012-
for (i = 0; i < numInherits; i++)
1013-
{
1014-
if (inhinfo[i].inhrelid == oid)
1015-
numParents++;
1016-
}
1017-
1018-
self->numParents = numParents;
1019-
1020-
if (numParents > 0)
1021-
{
1022-
self->parents = pg_malloc_array(TableInfo *, numParents);
1023-
j = 0;
1024-
for (i = 0; i < numInherits; i++)
1025-
{
1026-
if (inhinfo[i].inhrelid == oid)
1027-
{
1028-
TableInfo *parent;
1029-
1030-
parent = findTableByOid(inhinfo[i].inhparent);
1031-
if (parent == NULL)
1032-
pg_fatal("failed sanity check, parent OID %u of table \"%s\" (OID %u) not found",
1033-
inhinfo[i].inhparent,
1034-
self->dobj.name,
1035-
oid);
1036-
self->parents[j++] = parent;
1037-
}
1038-
}
1039-
}
1040-
else
1041-
self->parents = NULL;
1042-
}
1043-
10441022
/*
10451023
* parseOidArray
10461024
* parse a string of numbers delimited by spaces into a character array

src/bin/pg_dump/pg_dump.c

-3
Original file line numberDiff line numberDiff line change
@@ -16180,9 +16180,6 @@ dumpTableAttach(Archive *fout, const TableAttachInfo *attachinfo)
1618016180
if (dopt->dataOnly)
1618116181
return;
1618216182

16183-
if (!(attachinfo->partitionTbl->dobj.dump & DUMP_COMPONENT_DEFINITION))
16184-
return;
16185-
1618616183
q = createPQExpBuffer();
1618716184

1618816185
if (!fout->is_prepared[PREPQUERY_DUMPTABLEATTACH])

src/bin/pg_dump/pg_dump.h

+3-2
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,9 @@ typedef struct _tableInfo
320320
bool ispartition; /* is table a partition? */
321321
bool unsafe_partitions; /* is it an unsafe partitioned table? */
322322

323+
int numParents; /* number of (immediate) parent tables */
324+
struct _tableInfo **parents; /* TableInfos of immediate parents */
325+
323326
/*
324327
* These fields are computed only if we decide the table is interesting
325328
* (it's either a table to dump, or a direct parent of a dumpable table).
@@ -351,8 +354,6 @@ typedef struct _tableInfo
351354
/*
352355
* Stuff computed only for dumpable tables.
353356
*/
354-
int numParents; /* number of (immediate) parent tables */
355-
struct _tableInfo **parents; /* TableInfos of immediate parents */
356357
int numIndexes; /* number of indexes */
357358
struct _indxInfo *indexes; /* indexes */
358359
struct _tableDataInfo *dataObj; /* TableDataInfo, if dumping its data */

0 commit comments

Comments
 (0)