Re: logical decoding and replication of sequences

Looking at 0003,

On 2021-Dec-14, Tomas Vondra wrote:

> diff --git a/doc/src/sgml/ref/alter_publication.sgml b/doc/src/sgml/ref/alter_publication.sgml
> index bb4ef5e5e22..4d166ad3f9c 100644
> --- a/doc/src/sgml/ref/alter_publication.sgml
> +++ b/doc/src/sgml/ref/alter_publication.sgml
> @@ -31,7 +31,9 @@ ALTER PUBLICATION <replaceable class="parameter">name</replaceable> RENAME TO <r
> <phrase>where <replaceable class="parameter">publication_object</replaceable> is one of:</phrase>
>
> TABLE [ ONLY ] <replaceable class="parameter">table_name</replaceable> [ * ] [, ... ]
> + SEQUENCE <replaceable class="parameter">sequence_name</replaceable> [ * ] [, ... ]
> ALL TABLES IN SCHEMA { <replaceable class="parameter">schema_name</replaceable> | CURRENT_SCHEMA } [, ... ]
> + ALL SEQUENCE IN SCHEMA { <replaceable class="parameter">schema_name</replaceable> | CURRENT_SCHEMA } [, ... ]

Note that this says ALL SEQUENCE; I think it should be ALL SEQUENCES.

> diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
> index 3d4dd43e47b..f037c17985b 100644
> --- a/src/backend/parser/gram.y
> +++ b/src/backend/parser/gram.y
> @@ -9762,6 +9762,26 @@ PublicationObjSpec:
...
> + | ALL SEQUENCE IN_P SCHEMA ColId
> + {
> + $$ = makeNode(PublicationObjSpec);
> + $$->pubobjtype = PUBLICATIONOBJ_SEQUENCE_IN_SCHEMA;
> + $$->name = $5;
> + $$->location = @5;
> + }
> + | ALL SEQUENCES IN_P SCHEMA CURRENT_SCHEMA
> + {
> + $$ = makeNode(PublicationObjSpec);
> + $$->pubobjtype = PUBLICATIONOBJ_SEQUENCE_IN_CUR_SCHEMA;
> + $$->location = @5;
> + }

And here you have ALL SEQUENCE in one spot and ALL SEQUENCES in the
other.

BTW I think these enum values should use the plural too,
PUBLICATIONOBJ_SEQUENCES_IN_CUR_SCHEMA (not SEQUENCE). I suppose you
copied from PUBLICATIONOBJ_TABLE_IN_CUR_SCHEMA, but that too seems to be
a mistake: should be PUBLICATIONOBJ_TABLES_IN_CUR_SCHEMA.

> @@ -10097,6 +10117,12 @@ UnlistenStmt:
> }
> ;
>
> +/*
> + * FIXME
> + *
> + * opt_publication_for_sequences and publication_for_sequences should be
> + * copies for sequences
> + */

Not sure if this FIXME is relevant or should just be removed.

> @@ -10105,6 +10131,12 @@ UnlistenStmt:
> * BEGIN / COMMIT / ROLLBACK
> * (also older versions END / ABORT)
> *
> + * ALTER PUBLICATION name ADD SEQUENCE sequence [, sequence2]
> + *
> + * ALTER PUBLICATION name DROP SEQUENCE sequence [, sequence2]
> + *
> + * ALTER PUBLICATION name SET SEQUENCE sequence [, sequence2]
> + *

This comment addition seems misplaced?

> diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
> index 2f412ca3db3..e30bf7b1b55 100644
> --- a/src/bin/psql/tab-complete.c
> +++ b/src/bin/psql/tab-complete.c
> @@ -1647,13 +1647,13 @@ psql_completion(const char *text, int start, int end)
> COMPLETE_WITH("ADD", "DROP", "OWNER TO", "RENAME TO", "SET");
> /* ALTER PUBLICATION <name> ADD */
> else if (Matches("ALTER", "PUBLICATION", MatchAny, "ADD"))
> - COMPLETE_WITH("ALL TABLES IN SCHEMA", "TABLE");
> + COMPLETE_WITH("ALL TABLES IN SCHEMA", "TABLE|SEQUENCE");
> /* ALTER PUBLICATION <name> DROP */
> else if (Matches("ALTER", "PUBLICATION", MatchAny, "DROP"))
> - COMPLETE_WITH("ALL TABLES IN SCHEMA", "TABLE");
> + COMPLETE_WITH("ALL TABLES IN SCHEMA", "TABLE|SEQUENCE");
> /* ALTER PUBLICATION <name> SET */
> else if (Matches("ALTER", "PUBLICATION", MatchAny, "SET"))
> - COMPLETE_WITH("(", "ALL TABLES IN SCHEMA", "TABLE");
> + COMPLETE_WITH("(", "ALL TABLES IN SCHEMA", "TABLE|SEQUENCE");

I think you should also add "ALL SEQUENCES IN SCHEMA" to these lists.

> else if (Matches("ALTER", "PUBLICATION", MatchAny, "ADD|DROP|SET", "ALL", "TABLES", "IN", "SCHEMA"))

... and perhaps make this "ALL", "TABLES|SEQUENCES", "IN", "SCHEMA".

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"Nunca confiaré en un traidor. Ni siquiera si el traidor lo he creado yo"
(Barón Vladimir Harkonnen)