Re: Pluggable Storage - Andres's take

On Sat, 12 Jan 2019 at 18:11, Dmitry Dolgov <9erthalion6(at)gmail(dot)com> wrote:
> > On Sat, Jan 12, 2019 at 1:44 AM Andres Freund <andres(at)anarazel(dot)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