Re: Pluggable Storage - Andres's take

> On Sat, Jan 12, 2019 at 1:44 AM Andres Freund <andres(at)anarazel(dot)de> wrote:
>
> > + appendPQExpBuffer(cmd, "SET default_table_access_method = %s;", tableam);
>
> This needs escaping, at the very least with "", but better with proper
> routines for dealing with identifiers.

Thanks for noticing, fixed.

> > @@ -5914,7 +5922,7 @@ getTables(Archive *fout, int *numTables)
> > "tc.relfrozenxid AS tfrozenxid, "
> > "tc.relminmxid AS tminmxid, "
> > "c.relpersistence, c.relispopulated, "
> > - "c.relreplident, c.relpages, "
> > + "c.relreplident, c.relpages, am.amname AS amname, "
>
> That AS doesn't do anything, does it?

Rigth, I've renamed it few times and forgot to get rid of it. Removed.

>
> > /* 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 though 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.