> 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.
0001-ArchiveOpts-structure.patch
Description: Binary data