Обсуждение: tableam options for pg_dump/ALTER/LIKE

I made these casual comments.  If there's any agreement on their merit, it'd be
nice to implement at least the first for v13.

In <20190818193533.GL11185@telsasoft.com>, I wrote: 
>  . What do you think about pg_restore --no-tableam; similar to 
>    --no-tablespaces, it would allow restoring a table to a different AM:
>    PGOPTIONS='-c default_table_access_method=zedstore' pg_restore --no-tableam ./pg_dump.dat -d postgres
>    Otherwise, the dump says "SET default_table_access_method=heap", which
>    overrides any value from PGOPTIONS and precludes restoring to new AM.

That appears to be a trivial variation on no-tablespace:

        /* do nothing in --no-tablespaces mode */
        if (ropt->noTablespace)
                return;

>  . it'd be nice if there was an ALTER TABLE SET ACCESS METHOD, to allow
>    migrating data.  Otherwise I think the alternative is:
>     begin; lock t;
>     CREATE TABLE new_t LIKE (t INCLUDING ALL EXCLUDING INDEXES) USING (zedstore);
>     INSERT INTO new_t SELECT * FROM t;
>     for index; do CREATE INDEX...; done
>     DROP t; RENAME new_t (and all its indices). attach/inherit, etc.
>     commit;

Ideally that would allow all at once various combinations of altering
tablespace, changing AM, clustering, and reindexing, like what's discussed
here:
https://www.postgresql.org/message-id/flat/8a8f5f73-00d3-55f8-7583-1375ca8f6a91@postgrespro.ru

>  . Speaking of which, I think LIKE needs a new option for ACCESS METHOD, which
>    is otherwise lost.

I first suggested this a couple years ago.
Is it desirable to implement in pg_dump and pg_restore ?
It'd be just like --tablespace.

On Tue, Jan 28, 2020 at 07:33:17AM -0600, Justin Pryzby wrote:
> I made these casual comments.  If there's any agreement on their merit, it'd be
> nice to implement at least the first for v13.
> 
> In <20190818193533.GL11185@telsasoft.com>, I wrote: 
> >  . What do you think about pg_restore --no-tableam; similar to 
> >    --no-tablespaces, it would allow restoring a table to a different AM:
> >    PGOPTIONS='-c default_table_access_method=zedstore' pg_restore --no-tableam ./pg_dump.dat -d postgres
> >    Otherwise, the dump says "SET default_table_access_method=heap", which
> >    overrides any value from PGOPTIONS and precludes restoring to new AM.
> 
> That appears to be a trivial variation on no-tablespace:
> 
>         /* do nothing in --no-tablespaces mode */
>         if (ropt->noTablespace)
>                 return;
...

I forgot but had actually implemented this 6 months ago.

@cfbot: rebased

On Mon, Jan 03, 2022 at 03:44:24PM -0600, Justin Pryzby wrote:
> @cfbot: rebased

Hmm.  This could be useful to provide more control in some logical
reload scenarios, so I'd agree to provide this switch.  I'll look at
the patch later..
--
Michael

Hi,

On 2022-01-03 15:44:24 -0600, Justin Pryzby wrote:
> @cfbot: rebased

> From 69ae2ed5d00a97d351e1f6c45a9e406f33032898 Mon Sep 17 00:00:00 2001
> From: Justin Pryzby <pryzbyj@telsasoft.com>
> Date: Sun, 7 Mar 2021 19:35:37 -0600
> Subject: [PATCH] Add pg_dump/restore --no-table-am..
> 
> This was for some reason omitted from 3b925e905.

Seems the docs changes aren't quite right?

https://cirrus-ci.com/task/5864769860141056?logs=docs_build#L344

[02:43:01.356] ref/pg_dump.sgml:1162: parser error : Opening and ending tag mismatch: varlistentry line 934 and
variablelist
[02:43:01.356]     </variablelist>
[02:43:01.356]                    ^
....

> +     <varlistentry>
> +     <varlistentry>

Yup...

Greetings,

Andres Freund

On Mon, Jan 03, 2022 at 03:44:24PM -0600, Justin Pryzby wrote:
> +     <varlistentry>
> +     <varlistentry>
> +      <term><option>--no-table-am</option></term>
> +      <listitem>
> +       <para>
> +        Do not output commands to select table access methods.
> +        With this option, all objects will be created with whichever
> +        table access method is the default during restore.
> +       </para>

Hmm.  --no-table-am may not be the best choice.  Should this be called
--no-table-access-method instead?

> -    no_toast_compression => {
> -        dump_cmd => [
> -            'pg_dump', '--no-sync',
> -            "--file=$tempdir/no_toast_compression.sql",
> -            '--no-toast-compression', 'postgres',
> -        ],
> -    },

Why is this command moved down?
--
Michael

On Tue, Jan 11, 2022 at 04:50:23PM +0900, Michael Paquier wrote:
> On Mon, Jan 03, 2022 at 03:44:24PM -0600, Justin Pryzby wrote:
> > +     <varlistentry>
> > +     <varlistentry>
> > +      <term><option>--no-table-am</option></term>
> > +      <listitem>
> > +       <para>
> > +        Do not output commands to select table access methods.
> > +        With this option, all objects will be created with whichever
> > +        table access method is the default during restore.
> > +       </para>
> 
> Hmm.  --no-table-am may not be the best choice.  Should this be called
> --no-table-access-method instead?

I suppose you're right - I had previously renamed it from no-tableam.

> > -    no_toast_compression => {
> > -        dump_cmd => [
> > -            'pg_dump', '--no-sync',
> > -            "--file=$tempdir/no_toast_compression.sql",
> > -            '--no-toast-compression', 'postgres',
> > -        ],
> > -    },
> 
> Why is this command moved down?

Because it looks like this is intended to be mostly alphabetical, but that
wasn't preserved by 63db0ac3f.  It's most apparent in "my %full_runs".

The same could be said of no-privs, defaults_custom_format, pg_dumpall_globals,
section_data, but they've been that way forever.

-- 
Justin

On Tue, Jan 11, 2022 at 10:09:07PM -0600, Justin Pryzby wrote:
> I suppose you're right - I had previously renamed it from no-tableam.

Thanks for the new version.  I have noticed that support for the
option with pg_dumpall was missing, but that looks useful to me like
the other switches.

> Because it looks like this is intended to be mostly alphabetical, but that
> wasn't preserved by 63db0ac3f.  It's most apparent in "my %full_runs".

Sure.  Now I am not sure that this is worth poking at if we don't
change the back-branches, as this could cause conflicts.  So I have
left this change out at the end.

And, done.
--
Michael

On Mon, Jan 17, 2022 at 02:55:58PM +0900, Michael Paquier wrote:
> On Tue, Jan 11, 2022 at 10:09:07PM -0600, Justin Pryzby wrote:
> > I suppose you're right - I had previously renamed it from no-tableam.
> 
> Thanks for the new version.  I have noticed that support for the
> option with pg_dumpall was missing, but that looks useful to me like
> the other switches.

I saw that you added it to pg_dumpall.  But there's a typo in --help:

diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 1cab0dfdc75..94852e7cdbb 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -655,3 +655,3 @@ help(void)
     printf(_("  --no-sync                    do not wait for changes to be written safely to disk\n"));
-    printf(_("  --no-tables-access-method    do not dump table access methods\n"));
+    printf(_("  --no-table-access-method     do not dump table access methods\n"));
     printf(_("  --no-tablespaces             do not dump tablespace assignments\n"));

Feel free to leave it for now, and I'll add it to my typos branch.

> And, done.

Thanks!

-- 
Justin

On Mon, Jan 17, 2022 at 12:20:07AM -0600, Justin Pryzby wrote:
> I saw that you added it to pg_dumpall.  But there's a typo in --help:

Thanks, fixed.
--
Michael