Re: ArchiveEntry optional arguments refactoring

> On 2019-01-17 09:29:04 -0800, Andres Freund wrote:
> On 2019-01-17 10:23:39 -0500, Tom Lane wrote:
> > I don't buy the argument that this would move the goalposts in terms
> > of how much work it is to add a new argument. You'd still end up
> > touching every call site.
>
> Why? A lot of arguments that'd be potentially added or removed would not
> be set by each callsites.
>
> If you e.g. look at
>
> you can see that a lot of changes where like
> ArchiveEntry(fout, nilCatalogId, createDumpId(),
> "pg_largeobject", NULL, NULL, "",
> - false, "pg_largeobject", SECTION_PRE_DATA,
> + "pg_largeobject", SECTION_PRE_DATA,
> loOutQry->data, "", NULL,
> NULL, 0,
> NULL, NULL);
>
> i.e. just removing a 'false' argument. In like 70+ callsites. With the
> above scheme, we'd instead just have removed a single .withoids = true,
> from dumpTableSchema()'s ArchiveEntry() call.

To make this discussion a bit more specific, I've created a patch of how it can
look like. All the arguments, except Archive, CatalogId and DumpId I've moved
into the ArchiveOpts structure. Not all of them could be empty before, but
anyway it seems better for consistency and readability. Some of the arguments
had empty string as a default value, I haven't changed anything here yet
(although this mixture of NULL and "" in ArchiveEntry looks a bit confusing).

As Andres mentioned above, for 578b229718e / the WITH OID removal and pg_dump
modification from pluggable storage thread, this patch reduces number of
changes, related to ArchiveEntry, from 70+ to just one.