Обсуждение: BUG #6699: pg_restore with -j -- doesn't restore view that groups by primary key
BUG #6699: pg_restore with -j -- doesn't restore view that groups by primary key
От
joe@tanga.com
Дата:
The following bug has been logged on the website: Bug reference: 6699 Logged by: Joe Van Dyk Email address: joe@tanga.com PostgreSQL version: 9.1.4 Operating system: OSX Description:=20=20=20=20=20=20=20=20 $ pg_restore -O -j 4 ~/tanga.dump -d tanga_dev_full_backup=20 pg_restore: [archiver (db)] Error while PROCESSING TOC: pg_restore: [archiver (db)] Error from TOC entry 417; 1259 66296 VIEW site_channels monkey pg_restore: [archiver (db)] could not execute query: ERROR: column "channels.start_at" must appear in the GROUP BY clause or be used in an aggregate function LINE 2: SELECT channels.id, channels.start_at, channels.end_at, ... ^ Command was: CREATE VIEW site_channels AS SELECT channels.id, channels.start_at, channels.end_at, channels.title, channels.descriptio... site_channels view definition: View definition: SELECT channels.id, channels.start_at, channels.end_at, channels.title FROM channels LEFT JOIN channels_products cp ON cp.channel_id =3D channels.id LEFT JOIN buyable_products bp ON bp.id =3D cp.product_id GROUP BY channels.id; channels.id is a primary key.
On Tue, Jun 19, 2012 at 07:49:20PM +0000, joe@tanga.com wrote: > The following bug has been logged on the website: > > Bug reference: 6699 > Logged by: Joe Van Dyk > Email address: joe@tanga.com > PostgreSQL version: 9.1.4 > Operating system: OSX > Description: > > $ pg_restore -O -j 4 ~/tanga.dump -d tanga_dev_full_backup > > pg_restore: [archiver (db)] Error while PROCESSING TOC: > pg_restore: [archiver (db)] Error from TOC entry 417; 1259 66296 VIEW > site_channels monkey > pg_restore: [archiver (db)] could not execute query: ERROR: column > "channels.start_at" must appear in the GROUP BY clause or be used in an > aggregate function > LINE 2: SELECT channels.id, channels.start_at, channels.end_at, ... > ^ > Command was: CREATE VIEW site_channels AS > SELECT channels.id, channels.start_at, channels.end_at, channels.title, > channels.descriptio... > > > > site_channels view definition: > > View definition: > SELECT channels.id, channels.start_at, channels.end_at, channels.title > FROM channels > LEFT JOIN channels_products cp ON cp.channel_id = channels.id > LEFT JOIN buyable_products bp ON bp.id = cp.product_id > GROUP BY channels.id; > > channels.id is a primary key. Attached is a test case to reproduce the problem, courtesy of the original reporter. > > > -- > Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-bugs -Ryan
Вложения
Re: BUG #6699: pg_restore with -j -- doesn't restore view that groups by primary key
От
Alvaro Herrera
Дата:
Excerpts from Ryan Kelly's message of mar jun 19 16:20:58 -0400 2012: > On Tue, Jun 19, 2012 at 07:49:20PM +0000, joe@tanga.com wrote: > > View definition: > > SELECT channels.id, channels.start_at, channels.end_at, channels.title > > FROM channels > > LEFT JOIN channels_products cp ON cp.channel_id =3D channels.id > > LEFT JOIN buyable_products bp ON bp.id =3D cp.product_id > > GROUP BY channels.id; > >=20 > > channels.id is a primary key. >=20 > Attached is a test case to reproduce the problem, courtesy of the > original reporter. The reason this doesn't work is that the primary key is not defined until later in the restore process. I think the fix is to make the view dependant on the primary key in the dump file. --=20 =C3=81lvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Alvaro Herrera <alvherre@commandprompt.com> writes: > Excerpts from Ryan Kelly's message of mar jun 19 16:20:58 -0400 2012: >> On Tue, Jun 19, 2012 at 07:49:20PM +0000, joe@tanga.com wrote: >>> SELECT channels.id, channels.start_at, channels.end_at, channels.title >>> FROM channels >>> LEFT JOIN channels_products cp ON cp.channel_id = channels.id >>> LEFT JOIN buyable_products bp ON bp.id = cp.product_id >>> GROUP BY channels.id; > The reason this doesn't work is that the primary key is not defined > until later in the restore process. > I think the fix is to make the view dependant on the primary key in the > dump file. Hmm ... check_functional_grouping does add the PK's OID to the query's constraintDeps list. Apparently we're losing that dependency knowledge somewhere between the parser and pg_dump? regards, tom lane
I wrote: > Hmm ... check_functional_grouping does add the PK's OID to the query's > constraintDeps list. Apparently we're losing that dependency knowledge > somewhere between the parser and pg_dump? I looked into this a bit. The dependency does exist in pg_depend, but it is shown as a dependency from the view's _RETURN rule to the constraint, not as a dependency of the view itself. Using the simplified example create table t1 (f1 int primary key, f2 text); create view v1 as select * from t1 group by f1; what you get from "pg_dump -Fc | pg_restore -l -v" is ; Selected TOC Entries: ; 1923; 0 0 ENCODING - ENCODING 1924; 0 0 STDSTRINGS - STDSTRINGS 1925; 1262 41967 DATABASE - refbug postgres 5; 2615 2200 SCHEMA - public postgres 1926; 0 0 COMMENT - SCHEMA public postgres ; depends on: 5 1927; 0 0 ACL - public postgres ; depends on: 5 170; 3079 11727 EXTENSION - plpgsql 1928; 0 0 COMMENT - EXTENSION plpgsql ; depends on: 170 168; 1259 41968 TABLE public t1 postgres ; depends on: 5 1921; 2606 41975 CONSTRAINT public t1_pkey postgres ; depends on: 168 168 169; 1259 41976 VIEW public v1 postgres ; depends on: 1919 5 1922; 0 41968 TABLE DATA public t1 postgres ; depends on: 168 So the view is shown as depending on "object 1919", which is nowhere to be seen, because it is the _RETURN rule which did not get dumped separately. There is therefore no way at all for pg_restore to know that the view has to be restored after the constraint. (pg_dump does know that, since it was working with full dependency info, which is why the constraint comes first in the dump order. But the info isn't exposed where pg_restore can see it.) Clearly, this is a bug in the way pg_dump emits dependency info. It never mattered before, but parallel pg_restore really needs accurate dependencies. We could possibly hack something for the special case of rules, but I don't think this would be the last time we hear about this type of issue. I'm inclined to think that the best solution would be to add generic logic to pg_dump that "looks through" any dependency references to objects that are not going to be dumped, and replaces them with the IDs of any objects they depend on that are going to be dumped. regards, tom lane
I wrote: > We could possibly hack something for the special case of rules, but > I don't think this would be the last time we hear about this type of > issue. I'm inclined to think that the best solution would be to add > generic logic to pg_dump that "looks through" any dependency references > to objects that are not going to be dumped, and replaces them with the > IDs of any objects they depend on that are going to be dumped. I wrote a trial patch that does things that way (attached) and it appears to work and solve the problem. However, it's not committable as-is because it breaks the build of pg_restore: ld: Unsatisfied symbols: findObjectByDumpId (code) since findObjectByDumpId is in common.c which isn't linked into pg_restore. I guess a brute force solution would be to link it, but probably we ought to think about refactoring the code to avoid that. I'm not coming up with any very nice ideas about exactly how to refactor, though. Thoughts? regards, tom lane diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index aa9c8eed5dafabd49429874d973e22abe93d5294..2db67d6fd3d5616944ff0b086aa606af11e9eed4 100644 *** a/src/bin/pg_dump/pg_backup_archiver.c --- b/src/bin/pg_dump/pg_backup_archiver.c *************** static void _becomeUser(ArchiveHandle *A *** 129,134 **** --- 129,135 ---- static void _becomeOwner(ArchiveHandle *AH, TocEntry *te); static void _selectOutputSchema(ArchiveHandle *AH, const char *schemaName); static void _selectTablespace(ArchiveHandle *AH, const char *tablespace); + static void writeDependencies(ArchiveHandle *AH, const DumpId *dependencies, int nDeps); static void processEncodingEntry(ArchiveHandle *AH, TocEntry *te); static void processStdStringsEntry(ArchiveHandle *AH, TocEntry *te); static teReqs _tocEntryRequired(TocEntry *te, teSection curSection, RestoreOptions *ropt); *************** WriteToc(ArchiveHandle *AH) *** 2144,2150 **** TocEntry *te; char workbuf[32]; int tocCount; - int i; /* count entries that will actually be dumped */ tocCount = 0; --- 2145,2150 ---- *************** WriteToc(ArchiveHandle *AH) *** 2184,2195 **** WriteStr(AH, te->withOids ? "true" : "false"); /* Dump list of dependencies */ ! for (i = 0; i < te->nDeps; i++) ! { ! sprintf(workbuf, "%d", te->dependencies[i]); ! WriteStr(AH, workbuf); ! } ! WriteStr(AH, NULL); /* Terminate List */ if (AH->WriteExtraTocPtr) (*AH->WriteExtraTocPtr) (AH, te); --- 2184,2191 ---- WriteStr(AH, te->withOids ? "true" : "false"); /* Dump list of dependencies */ ! writeDependencies(AH, te->dependencies, te->nDeps); ! WriteStr(AH, NULL); /* list terminator */ if (AH->WriteExtraTocPtr) (*AH->WriteExtraTocPtr) (AH, te); *************** ReadToc(ArchiveHandle *AH) *** 2353,2358 **** --- 2349,2400 ---- } } + /* + * Dump an object's dependencies in a usable form, ie, referencing only + * objects that are included in the dump. We need to make this distinction + * since, for example, a view will depend on its _RETURN rule while the + * _RETURN rule will depend on other objects --- but the rule will not appear + * as a separate object in the dump. We have to look through the rule to get + * a useful representation of the view's dependencies. + * + * We rely here on the assumption that sortDumpableObjects already broke any + * dependency loops, else we might recurse infinitely. + */ + static void + writeDependencies(ArchiveHandle *AH, const DumpId *dependencies, int nDeps) + { + char workbuf[32]; + int i; + + for (i = 0; i < nDeps; i++) + { + DumpId depid = dependencies[i]; + TocEntry *depte = getTocEntryByDumpId(AH, depid); + + if (depte && + (depte->reqs & (REQ_SCHEMA | REQ_DATA | REQ_SPECIAL)) != 0) + { + /* Object will be dumped, so just reference it as a dependency */ + /* Dependency IDs are printed as strings for historical reasons */ + sprintf(workbuf, "%d", depid); + WriteStr(AH, workbuf); + } + else + { + /* + * Object will not be dumped, so recursively consider its deps. + * Here we must resort to the underlying DumpableObject data + * structures, since often there will be no TocEntry for such + * an object. + */ + DumpableObject *dobj = findObjectByDumpId(depid); + + if (dobj) + writeDependencies(AH, dobj->dependencies, dobj->nDeps); + } + } + } + static void processEncodingEntry(ArchiveHandle *AH, TocEntry *te) {
I wrote: > 168; 1259 41968 TABLE public t1 postgres > ; depends on: 5 > 1921; 2606 41975 CONSTRAINT public t1_pkey postgres > ; depends on: 168 168 > 169; 1259 41976 VIEW public v1 postgres > ; depends on: 1919 5 > 1922; 0 41968 TABLE DATA public t1 postgres > ; depends on: 168 BTW, there's another pretty unpleasant thing going on here, which is that the t1_pkey constraint is getting hoisted up to before t1's table data because it is a dependency of v1. That means the index will be created before the data is loaded, which is not what we want. Parallel pg_restore actually has a hack that should work around that, namely repoint_table_dependencies(). That doesn't help for plain serial restores though. I'm thinking we really ought to bite the bullet and do something comparable to repoint_table_dependencies() at an appropriate point in pg_dump, so that the dependencies are sane to begin with. regards, tom lane
I wrote: >> We could possibly hack something for the special case of rules, but >> I don't think this would be the last time we hear about this type of >> issue. I'm inclined to think that the best solution would be to add >> generic logic to pg_dump that "looks through" any dependency references >> to objects that are not going to be dumped, and replaces them with the >> IDs of any objects they depend on that are going to be dumped. > I wrote a trial patch that does things that way (attached) and it > appears to work and solve the problem. However, it's not committable > as-is because it breaks the build of pg_restore: > ld: Unsatisfied symbols: > findObjectByDumpId (code) > since findObjectByDumpId is in common.c which isn't linked into > pg_restore. I guess a brute force solution would be to link it, > but probably we ought to think about refactoring the code to avoid > that. Actually, that brute force solution doesn't work at all, because common.c contains many call-backs into pg_dump.c, and we certainly don't want to link pg_dump.c into pg_restore. I thought for a little bit about breaking common.c into two pieces, but am not excited about doing major refactoring on the spur of the moment. (Mind you, this code probably *needs* wholesale refactoring; I just don't want to do it as part of a bug fix.) Another problem I noticed after further thought is that in a partial or data-only dump, this patch could rearrange dependencies that pg_restore has hardwired knowledge of, such as the dependency of a TABLE DATA item on its table. I'm not certain that that would break anything, but it seems pretty darn dangerous. I think that we don't want to rearrange any dependencies that are "built in" by pg_dump itself rather than being obtained from pg_depend --- this includes the TABLE DATA case as well as COMMENT, ACL, and SECURITY LABEL items' dependencies on their parent objects. So attached is another try that puts the work into pg_dump.c. There are a couple things I don't especially like, but don't see an easy way to improve on: * This code would be the first in pg_dump.c that looks through Archive to ArchiveHandle. Although we've muttered before that that distinction is useless, doing this here still seems a bit out of place. But we can't put it in pg_backup_archiver.c because of the findObjectByDumpId dependence, so I see no better answer. * To handle the "built in" dependency problem mentioned above, I introduced a convention that only "built in" dependencies should be explicitly passed to ArchiveEntry(); everything with regular dependencies should pass NULL/0, and then we build the real dependencies at the very end of the archive construction process. This is annoying because it's easy to do the wrong thing: if you forget and use the old style of passing the DumpableObject's dependencies to ArchiveEntry, it will *appear* to work, and only break when/if one of these indirection cases comes up. A less error-prone convention would be nice, but I can't think of a good one offhand. Another thing that might be a showstopper is that this will only work as-is in HEAD and 9.2, because only since commit 4317e0246c645f60c39e6572644cff1cb03b4c65 do we have explicit advance marking of which TOC entries are going to get dumped. If we want to fix this problem this way in older branches, we're going to have to back-port parts of that commit. I don't actually see any alternative way to fix it, though. How concerned are we about making this work in released branches? Comments? regards, tom lane diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 5fde18921ac3c20f878fbe5ad22c60fabc13a916..247406364c734c918543a54dc37cb6b4fc188a2c 100644 *** a/src/bin/pg_dump/pg_dump.c --- b/src/bin/pg_dump/pg_dump.c *************** static void dumpACL(Archive *fout, Catal *** 210,215 **** --- 210,219 ---- const char *acls); static void getDependencies(Archive *fout); + static void BuildArchiveDependencies(Archive *fout); + static void findDumpableDependencies(ArchiveHandle *AH, DumpableObject *dobj, + DumpId **dependencies, int *nDeps, int *allocDeps); + static void getDomainConstraints(Archive *fout, TypeInfo *tyinfo); static void getTableData(TableInfo *tblinfo, int numTables, bool oids); static void makeTableDataInfo(TableInfo *tbinfo, bool oids); *************** main(int argc, char **argv) *** 753,758 **** --- 757,770 ---- SetArchiveRestoreOptions(fout, ropt); /* + * The archive's TOC entries are now marked as to which ones will + * actually be output, so we can set up their dependency lists properly. + * This isn't necessary for plain-text output, though. + */ + if (!plainText) + BuildArchiveDependencies(fout); + + /* * And finally we can do the actual output. * * Note: for non-plain-text output formats, the output file is written *************** dumpTableData(Archive *fout, TableDataIn *** 1558,1569 **** copyStmt = NULL; } ArchiveEntry(fout, tdinfo->dobj.catId, tdinfo->dobj.dumpId, tbinfo->dobj.name, tbinfo->dobj.namespace->dobj.name, NULL, tbinfo->rolname, false, "TABLE DATA", SECTION_DATA, "", "", copyStmt, ! tdinfo->dobj.dependencies, tdinfo->dobj.nDeps, dumpFn, tdinfo); destroyPQExpBuffer(copyBuf); --- 1570,1586 ---- copyStmt = NULL; } + /* + * Note: although the TableDataInfo is a full DumpableObject, we treat its + * dependency on its table as "special" and pass it to ArchiveEntry now. + * See comments for BuildArchiveDependencies. + */ ArchiveEntry(fout, tdinfo->dobj.catId, tdinfo->dobj.dumpId, tbinfo->dobj.name, tbinfo->dobj.namespace->dobj.name, NULL, tbinfo->rolname, false, "TABLE DATA", SECTION_DATA, "", "", copyStmt, ! &(tbinfo->dobj.dumpId), 1, dumpFn, tdinfo); destroyPQExpBuffer(copyBuf); *************** dumpBlob(Archive *fout, BlobInfo *binfo) *** 2247,2253 **** binfo->rolname, false, "BLOB", SECTION_PRE_DATA, cquery->data, dquery->data, NULL, ! binfo->dobj.dependencies, binfo->dobj.nDeps, NULL, NULL); /* set up tag for comment and/or ACL */ --- 2264,2270 ---- binfo->rolname, false, "BLOB", SECTION_PRE_DATA, cquery->data, dquery->data, NULL, ! NULL, 0, NULL, NULL); /* set up tag for comment and/or ACL */ *************** dumpDumpableObject(Archive *fout, Dumpab *** 7181,7187 **** dobj->name, NULL, NULL, "", false, "BLOBS", SECTION_DATA, "", "", NULL, ! dobj->dependencies, dobj->nDeps, dumpBlobs, NULL); break; } --- 7198,7204 ---- dobj->name, NULL, NULL, "", false, "BLOBS", SECTION_DATA, "", "", NULL, ! NULL, 0, dumpBlobs, NULL); break; } *************** dumpNamespace(Archive *fout, NamespaceIn *** 7228,7234 **** nspinfo->rolname, false, "SCHEMA", SECTION_PRE_DATA, q->data, delq->data, NULL, ! nspinfo->dobj.dependencies, nspinfo->dobj.nDeps, NULL, NULL); /* Dump Schema Comments and Security Labels */ --- 7245,7251 ---- nspinfo->rolname, false, "SCHEMA", SECTION_PRE_DATA, q->data, delq->data, NULL, ! NULL, 0, NULL, NULL); /* Dump Schema Comments and Security Labels */ *************** dumpExtension(Archive *fout, ExtensionIn *** 7346,7352 **** "", false, "EXTENSION", SECTION_PRE_DATA, q->data, delq->data, NULL, ! extinfo->dobj.dependencies, extinfo->dobj.nDeps, NULL, NULL); /* Dump Extension Comments and Security Labels */ --- 7363,7369 ---- "", false, "EXTENSION", SECTION_PRE_DATA, q->data, delq->data, NULL, ! NULL, 0, NULL, NULL); /* Dump Extension Comments and Security Labels */ *************** dumpEnumType(Archive *fout, TypeInfo *ty *** 7494,7500 **** tyinfo->rolname, false, "TYPE", SECTION_PRE_DATA, q->data, delq->data, NULL, ! tyinfo->dobj.dependencies, tyinfo->dobj.nDeps, NULL, NULL); /* Dump Type Comments and Security Labels */ --- 7511,7517 ---- tyinfo->rolname, false, "TYPE", SECTION_PRE_DATA, q->data, delq->data, NULL, ! NULL, 0, NULL, NULL); /* Dump Type Comments and Security Labels */ *************** dumpRangeType(Archive *fout, TypeInfo *t *** 7619,7625 **** tyinfo->rolname, false, "TYPE", SECTION_PRE_DATA, q->data, delq->data, NULL, ! tyinfo->dobj.dependencies, tyinfo->dobj.nDeps, NULL, NULL); /* Dump Type Comments and Security Labels */ --- 7636,7642 ---- tyinfo->rolname, false, "TYPE", SECTION_PRE_DATA, q->data, delq->data, NULL, ! NULL, 0, NULL, NULL); /* Dump Type Comments and Security Labels */ *************** dumpBaseType(Archive *fout, TypeInfo *ty *** 8001,8007 **** tyinfo->rolname, false, "TYPE", SECTION_PRE_DATA, q->data, delq->data, NULL, ! tyinfo->dobj.dependencies, tyinfo->dobj.nDeps, NULL, NULL); /* Dump Type Comments and Security Labels */ --- 8018,8024 ---- tyinfo->rolname, false, "TYPE", SECTION_PRE_DATA, q->data, delq->data, NULL, ! NULL, 0, NULL, NULL); /* Dump Type Comments and Security Labels */ *************** dumpDomain(Archive *fout, TypeInfo *tyin *** 8156,8162 **** tyinfo->rolname, false, "DOMAIN", SECTION_PRE_DATA, q->data, delq->data, NULL, ! tyinfo->dobj.dependencies, tyinfo->dobj.nDeps, NULL, NULL); /* Dump Domain Comments and Security Labels */ --- 8173,8179 ---- tyinfo->rolname, false, "DOMAIN", SECTION_PRE_DATA, q->data, delq->data, NULL, ! NULL, 0, NULL, NULL); /* Dump Domain Comments and Security Labels */ *************** dumpCompositeType(Archive *fout, TypeInf *** 8363,8369 **** tyinfo->rolname, false, "TYPE", SECTION_PRE_DATA, q->data, delq->data, NULL, ! tyinfo->dobj.dependencies, tyinfo->dobj.nDeps, NULL, NULL); --- 8380,8386 ---- tyinfo->rolname, false, "TYPE", SECTION_PRE_DATA, q->data, delq->data, NULL, ! NULL, 0, NULL, NULL); *************** dumpShellType(Archive *fout, ShellTypeIn *** 8535,8541 **** stinfo->baseType->rolname, false, "SHELL TYPE", SECTION_PRE_DATA, q->data, "", NULL, ! stinfo->dobj.dependencies, stinfo->dobj.nDeps, NULL, NULL); destroyPQExpBuffer(q); --- 8552,8558 ---- stinfo->baseType->rolname, false, "SHELL TYPE", SECTION_PRE_DATA, q->data, "", NULL, ! NULL, 0, NULL, NULL); destroyPQExpBuffer(q); *************** dumpProcLang(Archive *fout, ProcLangInfo *** 8709,8715 **** lanschema, NULL, plang->lanowner, false, "PROCEDURAL LANGUAGE", SECTION_PRE_DATA, defqry->data, delqry->data, NULL, ! plang->dobj.dependencies, plang->dobj.nDeps, NULL, NULL); /* Dump Proc Lang Comments and Security Labels */ --- 8726,8732 ---- lanschema, NULL, plang->lanowner, false, "PROCEDURAL LANGUAGE", SECTION_PRE_DATA, defqry->data, delqry->data, NULL, ! NULL, 0, NULL, NULL); /* Dump Proc Lang Comments and Security Labels */ *************** dumpFunc(Archive *fout, FuncInfo *finfo) *** 9296,9302 **** finfo->rolname, false, "FUNCTION", SECTION_PRE_DATA, q->data, delqry->data, NULL, ! finfo->dobj.dependencies, finfo->dobj.nDeps, NULL, NULL); /* Dump Function Comments and Security Labels */ --- 9313,9319 ---- finfo->rolname, false, "FUNCTION", SECTION_PRE_DATA, q->data, delqry->data, NULL, ! NULL, 0, NULL, NULL); /* Dump Function Comments and Security Labels */ *************** dumpCast(Archive *fout, CastInfo *cast) *** 9466,9472 **** "pg_catalog", NULL, "", false, "CAST", SECTION_PRE_DATA, defqry->data, delqry->data, NULL, ! cast->dobj.dependencies, cast->dobj.nDeps, NULL, NULL); /* Dump Cast Comments */ --- 9483,9489 ---- "pg_catalog", NULL, "", false, "CAST", SECTION_PRE_DATA, defqry->data, delqry->data, NULL, ! NULL, 0, NULL, NULL); /* Dump Cast Comments */ *************** dumpOpr(Archive *fout, OprInfo *oprinfo) *** 9700,9706 **** oprinfo->rolname, false, "OPERATOR", SECTION_PRE_DATA, q->data, delq->data, NULL, ! oprinfo->dobj.dependencies, oprinfo->dobj.nDeps, NULL, NULL); /* Dump Operator Comments */ --- 9717,9723 ---- oprinfo->rolname, false, "OPERATOR", SECTION_PRE_DATA, q->data, delq->data, NULL, ! NULL, 0, NULL, NULL); /* Dump Operator Comments */ *************** dumpOpclass(Archive *fout, OpclassInfo * *** 10207,10213 **** opcinfo->rolname, false, "OPERATOR CLASS", SECTION_PRE_DATA, q->data, delq->data, NULL, ! opcinfo->dobj.dependencies, opcinfo->dobj.nDeps, NULL, NULL); /* Dump Operator Class Comments */ --- 10224,10230 ---- opcinfo->rolname, false, "OPERATOR CLASS", SECTION_PRE_DATA, q->data, delq->data, NULL, ! NULL, 0, NULL, NULL); /* Dump Operator Class Comments */ *************** dumpOpfamily(Archive *fout, OpfamilyInfo *** 10520,10526 **** opfinfo->rolname, false, "OPERATOR FAMILY", SECTION_PRE_DATA, q->data, delq->data, NULL, ! opfinfo->dobj.dependencies, opfinfo->dobj.nDeps, NULL, NULL); /* Dump Operator Family Comments */ --- 10537,10543 ---- opfinfo->rolname, false, "OPERATOR FAMILY", SECTION_PRE_DATA, q->data, delq->data, NULL, ! NULL, 0, NULL, NULL); /* Dump Operator Family Comments */ *************** dumpCollation(Archive *fout, CollInfo *c *** 10609,10615 **** collinfo->rolname, false, "COLLATION", SECTION_PRE_DATA, q->data, delq->data, NULL, ! collinfo->dobj.dependencies, collinfo->dobj.nDeps, NULL, NULL); /* Dump Collation Comments */ --- 10626,10632 ---- collinfo->rolname, false, "COLLATION", SECTION_PRE_DATA, q->data, delq->data, NULL, ! NULL, 0, NULL, NULL); /* Dump Collation Comments */ *************** dumpConversion(Archive *fout, ConvInfo * *** 10708,10714 **** convinfo->rolname, false, "CONVERSION", SECTION_PRE_DATA, q->data, delq->data, NULL, ! convinfo->dobj.dependencies, convinfo->dobj.nDeps, NULL, NULL); /* Dump Conversion Comments */ --- 10725,10731 ---- convinfo->rolname, false, "CONVERSION", SECTION_PRE_DATA, q->data, delq->data, NULL, ! NULL, 0, NULL, NULL); /* Dump Conversion Comments */ *************** dumpAgg(Archive *fout, AggInfo *agginfo) *** 10945,10951 **** agginfo->aggfn.rolname, false, "AGGREGATE", SECTION_PRE_DATA, q->data, delq->data, NULL, ! agginfo->aggfn.dobj.dependencies, agginfo->aggfn.dobj.nDeps, NULL, NULL); /* Dump Aggregate Comments */ --- 10962,10968 ---- agginfo->aggfn.rolname, false, "AGGREGATE", SECTION_PRE_DATA, q->data, delq->data, NULL, ! NULL, 0, NULL, NULL); /* Dump Aggregate Comments */ *************** dumpTSParser(Archive *fout, TSParserInfo *** 11043,11049 **** "", false, "TEXT SEARCH PARSER", SECTION_PRE_DATA, q->data, delq->data, NULL, ! prsinfo->dobj.dependencies, prsinfo->dobj.nDeps, NULL, NULL); /* Dump Parser Comments */ --- 11060,11066 ---- "", false, "TEXT SEARCH PARSER", SECTION_PRE_DATA, q->data, delq->data, NULL, ! NULL, 0, NULL, NULL); /* Dump Parser Comments */ *************** dumpTSDictionary(Archive *fout, TSDictIn *** 11130,11136 **** dictinfo->rolname, false, "TEXT SEARCH DICTIONARY", SECTION_PRE_DATA, q->data, delq->data, NULL, ! dictinfo->dobj.dependencies, dictinfo->dobj.nDeps, NULL, NULL); /* Dump Dictionary Comments */ --- 11147,11153 ---- dictinfo->rolname, false, "TEXT SEARCH DICTIONARY", SECTION_PRE_DATA, q->data, delq->data, NULL, ! NULL, 0, NULL, NULL); /* Dump Dictionary Comments */ *************** dumpTSTemplate(Archive *fout, TSTemplate *** 11196,11202 **** "", false, "TEXT SEARCH TEMPLATE", SECTION_PRE_DATA, q->data, delq->data, NULL, ! tmplinfo->dobj.dependencies, tmplinfo->dobj.nDeps, NULL, NULL); /* Dump Template Comments */ --- 11213,11219 ---- "", false, "TEXT SEARCH TEMPLATE", SECTION_PRE_DATA, q->data, delq->data, NULL, ! NULL, 0, NULL, NULL); /* Dump Template Comments */ *************** dumpTSConfig(Archive *fout, TSConfigInfo *** 11324,11330 **** cfginfo->rolname, false, "TEXT SEARCH CONFIGURATION", SECTION_PRE_DATA, q->data, delq->data, NULL, ! cfginfo->dobj.dependencies, cfginfo->dobj.nDeps, NULL, NULL); /* Dump Configuration Comments */ --- 11341,11347 ---- cfginfo->rolname, false, "TEXT SEARCH CONFIGURATION", SECTION_PRE_DATA, q->data, delq->data, NULL, ! NULL, 0, NULL, NULL); /* Dump Configuration Comments */ *************** dumpForeignDataWrapper(Archive *fout, Fd *** 11398,11404 **** fdwinfo->rolname, false, "FOREIGN DATA WRAPPER", SECTION_PRE_DATA, q->data, delq->data, NULL, ! fdwinfo->dobj.dependencies, fdwinfo->dobj.nDeps, NULL, NULL); /* Handle the ACL */ --- 11415,11421 ---- fdwinfo->rolname, false, "FOREIGN DATA WRAPPER", SECTION_PRE_DATA, q->data, delq->data, NULL, ! NULL, 0, NULL, NULL); /* Handle the ACL */ *************** dumpForeignServer(Archive *fout, Foreign *** 11490,11496 **** srvinfo->rolname, false, "SERVER", SECTION_PRE_DATA, q->data, delq->data, NULL, ! srvinfo->dobj.dependencies, srvinfo->dobj.nDeps, NULL, NULL); /* Handle the ACL */ --- 11507,11513 ---- srvinfo->rolname, false, "SERVER", SECTION_PRE_DATA, q->data, delq->data, NULL, ! NULL, 0, NULL, NULL); /* Handle the ACL */ *************** dumpDefaultACL(Archive *fout, DefaultACL *** 11674,11680 **** daclinfo->defaclrole, false, "DEFAULT ACL", SECTION_NONE, q->data, "", NULL, ! daclinfo->dobj.dependencies, daclinfo->dobj.nDeps, NULL, NULL); destroyPQExpBuffer(tag); --- 11691,11697 ---- daclinfo->defaclrole, false, "DEFAULT ACL", SECTION_NONE, q->data, "", NULL, ! NULL, 0, NULL, NULL); destroyPQExpBuffer(tag); *************** dumpTableSchema(Archive *fout, TableInfo *** 12673,12679 **** (strcmp(reltypename, "TABLE") == 0) ? tbinfo->hasoids : false, reltypename, SECTION_PRE_DATA, q->data, delq->data, NULL, ! tbinfo->dobj.dependencies, tbinfo->dobj.nDeps, NULL, NULL); --- 12690,12696 ---- (strcmp(reltypename, "TABLE") == 0) ? tbinfo->hasoids : false, reltypename, SECTION_PRE_DATA, q->data, delq->data, NULL, ! NULL, 0, NULL, NULL); *************** dumpAttrDef(Archive *fout, AttrDefInfo * *** 12745,12751 **** tbinfo->rolname, false, "DEFAULT", SECTION_PRE_DATA, q->data, delq->data, NULL, ! adinfo->dobj.dependencies, adinfo->dobj.nDeps, NULL, NULL); destroyPQExpBuffer(q); --- 12762,12768 ---- tbinfo->rolname, false, "DEFAULT", SECTION_PRE_DATA, q->data, delq->data, NULL, ! NULL, 0, NULL, NULL); destroyPQExpBuffer(q); *************** dumpIndex(Archive *fout, IndxInfo *indxi *** 12847,12853 **** tbinfo->rolname, false, "INDEX", SECTION_POST_DATA, q->data, delq->data, NULL, ! indxinfo->dobj.dependencies, indxinfo->dobj.nDeps, NULL, NULL); } --- 12864,12870 ---- tbinfo->rolname, false, "INDEX", SECTION_POST_DATA, q->data, delq->data, NULL, ! NULL, 0, NULL, NULL); } *************** dumpConstraint(Archive *fout, Constraint *** 12968,12974 **** tbinfo->rolname, false, "CONSTRAINT", SECTION_POST_DATA, q->data, delq->data, NULL, ! coninfo->dobj.dependencies, coninfo->dobj.nDeps, NULL, NULL); } else if (coninfo->contype == 'f') --- 12985,12991 ---- tbinfo->rolname, false, "CONSTRAINT", SECTION_POST_DATA, q->data, delq->data, NULL, ! NULL, 0, NULL, NULL); } else if (coninfo->contype == 'f') *************** dumpConstraint(Archive *fout, Constraint *** 13001,13007 **** tbinfo->rolname, false, "FK CONSTRAINT", SECTION_POST_DATA, q->data, delq->data, NULL, ! coninfo->dobj.dependencies, coninfo->dobj.nDeps, NULL, NULL); } else if (coninfo->contype == 'c' && tbinfo) --- 13018,13024 ---- tbinfo->rolname, false, "FK CONSTRAINT", SECTION_POST_DATA, q->data, delq->data, NULL, ! NULL, 0, NULL, NULL); } else if (coninfo->contype == 'c' && tbinfo) *************** dumpConstraint(Archive *fout, Constraint *** 13036,13042 **** tbinfo->rolname, false, "CHECK CONSTRAINT", SECTION_POST_DATA, q->data, delq->data, NULL, ! coninfo->dobj.dependencies, coninfo->dobj.nDeps, NULL, NULL); } } --- 13053,13059 ---- tbinfo->rolname, false, "CHECK CONSTRAINT", SECTION_POST_DATA, q->data, delq->data, NULL, ! NULL, 0, NULL, NULL); } } *************** dumpConstraint(Archive *fout, Constraint *** 13072,13078 **** tyinfo->rolname, false, "CHECK CONSTRAINT", SECTION_POST_DATA, q->data, delq->data, NULL, ! coninfo->dobj.dependencies, coninfo->dobj.nDeps, NULL, NULL); } } --- 13089,13095 ---- tyinfo->rolname, false, "CHECK CONSTRAINT", SECTION_POST_DATA, q->data, delq->data, NULL, ! NULL, 0, NULL, NULL); } } *************** dumpSequence(Archive *fout, TableInfo *t *** 13334,13340 **** tbinfo->rolname, false, "SEQUENCE", SECTION_PRE_DATA, query->data, delqry->data, NULL, ! tbinfo->dobj.dependencies, tbinfo->dobj.nDeps, NULL, NULL); /* --- 13351,13357 ---- tbinfo->rolname, false, "SEQUENCE", SECTION_PRE_DATA, query->data, delqry->data, NULL, ! NULL, 0, NULL, NULL); /* *************** dumpTrigger(Archive *fout, TriggerInfo * *** 13600,13606 **** tbinfo->rolname, false, "TRIGGER", SECTION_POST_DATA, query->data, delqry->data, NULL, ! tginfo->dobj.dependencies, tginfo->dobj.nDeps, NULL, NULL); dumpComment(fout, labelq->data, --- 13617,13623 ---- tbinfo->rolname, false, "TRIGGER", SECTION_POST_DATA, query->data, delqry->data, NULL, ! NULL, 0, NULL, NULL); dumpComment(fout, labelq->data, *************** dumpRule(Archive *fout, RuleInfo *rinfo) *** 13721,13727 **** tbinfo->rolname, false, "RULE", SECTION_POST_DATA, cmd->data, delcmd->data, NULL, ! rinfo->dobj.dependencies, rinfo->dobj.nDeps, NULL, NULL); /* Dump rule comments */ --- 13738,13744 ---- tbinfo->rolname, false, "RULE", SECTION_POST_DATA, cmd->data, delcmd->data, NULL, ! NULL, 0, NULL, NULL); /* Dump rule comments */ *************** getDependencies(Archive *fout) *** 14028,14033 **** --- 14045,14160 ---- /* + * BuildArchiveDependencies - create dependency data for archive TOC entries + * + * The raw dependency data obtained by getDependencies() is not terribly + * useful in an archive dump, because in many cases there are dependency + * chains linking through objects that don't appear explicitly in the dump. + * For example, a view will depend on its _RETURN rule while the _RETURN rule + * will depend on other objects --- but the rule will not appear as a separate + * object in the dump. We need to adjust the view's dependencies to include + * whatever the rule depends on that is included in the dump. + * + * Just to make things more complicated, there are also "special" dependencies + * such as the dependency of a TABLE DATA item on its TABLE, which we must + * not rearrange because pg_restore knows that TABLE DATA only depends on + * its table. In these cases we must leave the dependencies strictly as-is + * even if they refer to not-to-be-dumped objects. + * + * To handle this, the convention is that "special" dependencies are created + * during ArchiveEntry calls, and an archive TOC item that has any such + * entries will not be touched here. Otherwise, we recursively search the + * DumpableObject data structures to build the correct dependencies for each + * archive TOC item. + */ + static void + BuildArchiveDependencies(Archive *fout) + { + ArchiveHandle *AH = (ArchiveHandle *) fout; + TocEntry *te; + + /* Scan all TOC entries in the archive */ + for (te = AH->toc->next; te != AH->toc; te = te->next) + { + DumpableObject *dobj; + DumpId *dependencies; + int nDeps; + int allocDeps; + + /* No need to process entries that will not be dumped */ + if (te->reqs == 0) + continue; + /* Ignore entries that already have "special" dependencies */ + if (te->nDeps > 0) + continue; + /* Otherwise, look up the item's original DumpableObject, if any */ + dobj = findObjectByDumpId(te->dumpId); + if (dobj == NULL) + continue; + /* No work if it has no dependencies */ + if (dobj->nDeps <= 0) + continue; + /* Set up work array */ + allocDeps = 64; + dependencies = (DumpId *) pg_malloc(allocDeps * sizeof(DumpId)); + nDeps = 0; + /* Recursively find all dumpable dependencies */ + findDumpableDependencies(AH, dobj, + &dependencies, &nDeps, &allocDeps); + /* And save 'em ... */ + if (nDeps > 0) + { + dependencies = (DumpId *) pg_realloc(dependencies, + nDeps * sizeof(DumpId)); + te->dependencies = dependencies; + te->nDeps = nDeps; + } + else + free(dependencies); + } + } + + /* Recursive search subroutine for BuildArchiveDependencies */ + static void + findDumpableDependencies(ArchiveHandle *AH, DumpableObject *dobj, + DumpId **dependencies, int *nDeps, int *allocDeps) + { + int i; + + for (i = 0; i < dobj->nDeps; i++) + { + DumpId depid = dobj->dependencies[i]; + + if (TocIDRequired(AH, depid) != 0) + { + /* Object will be dumped, so just reference it as a dependency */ + if (*nDeps >= *allocDeps) + { + *allocDeps *= 2; + *dependencies = (DumpId *) pg_realloc(*dependencies, + *allocDeps * sizeof(DumpId)); + } + (*dependencies)[*nDeps] = depid; + (*nDeps)++; + } + else + { + /* + * Object will not be dumped, so recursively consider its deps. + * We rely on the assumption that sortDumpableObjects already + * broke any dependency loops, else we might recurse infinitely. + */ + DumpableObject *otherdobj = findObjectByDumpId(depid); + + if (otherdobj) + findDumpableDependencies(AH, otherdobj, + dependencies, nDeps, allocDeps); + } + } + } + + + /* * selectSourceSchema - make the specified schema the active search path * in the source database. *
I wrote: >>> We could possibly hack something for the special case of rules, but >>> I don't think this would be the last time we hear about this type of >>> issue. BTW, after comparing the dependencies emitted by this patch to those of HEAD for the regression database, I am convinced that indeed view rules are not the only issue; we have got a boatload of problems of this ilk, for instance: * objects dependent on array types link to an array-type entry that is not in the dump, not to the base type that is * composite types link to their underlying "DO_TABLE" entry (ie, the pg_class entry for the composite type) and thus miss any dependencies on, say, column data types * objects dependent on extension members have dangling links; notably, every single plpgsql function fails to have a visible dependency on the plpgsql extension * foreign key constraints have dependencies on PK indexes, but if the PK index was declared as a constraint, it's not visible under its own ID in the dump, so the dependency dangles. The proposed patch fixes all these cases. I am not sure whether any of the first three are hazards for parallel pg_restore, but the last one is: AFAICS, it will result in all such FK constraints being restored serially at the end, because the parallel loop never manages to clear all of their dependencies. Surprising nobody's complained about that. regards, tom lane