On Sat, 12 Jan 2019 at 18:11, Dmitry Dolgov <9erthali...@gmail.com> wrote: > > On Sat, Jan 12, 2019 at 1:44 AM Andres Freund <and...@anarazel.de> wrote: > > > /* other fields were zeroed above */ > > > > > > @@ -9355,7 +9370,7 @@ dumpComment(Archive *fout, const char *type, const > > > char *name, > > > * post-data. > > > */ > > > ArchiveEntry(fout, nilCatalogId, createDumpId(), > > > - tag->data, namespace, NULL, owner, > > > + tag->data, namespace, NULL, owner, > > > NULL, > > > "COMMENT", SECTION_NONE, > > > query->data, "", NULL, > > > &(dumpId), 1, > > > > We really ought to move the arguments to a struct, so we don't generate > > quite as much useless diffs whenever we do a change around one of > > these... > > That's what I thought too. Maybe then I'll suggest a mini-patch to the master > to > refactor these arguments out into a separate struct, so we can leverage it > here.
Then for each of the calls, we would need to declare that structure variable (with = {0}) and assign required fields in that structure before passing it to ArchiveEntry(). But a major reason of ArchiveEntry() is to avoid doing this and instead conveniently pass those fields as parameters. This will cause unnecessary more lines of code. I think better way is to have an ArchiveEntry() function with limited number of parameters, and have an ArchiveEntryEx() with those extra parameters which are not needed in usual cases. E.g. we can have tablespace, tableam, dumpFn and dumpArg as those extra arguments of ArchiveEntryEx(), because most of the places these are passed as NULL. All future arguments would go in ArchiveEntryEx(). Comments ? -- Thanks, -Amit Khandekar EnterpriseDB Corporation The Postgres Database Company