Re: logical decoding and replication of sequences

Поиск
Список
Период
Сортировка
От Alvaro Herrera
Тема Re: logical decoding and replication of sequences
Дата
Msg-id 202112151642.h7cehn6zplxg@alvherre.pgsql
обсуждение исходный текст
Ответ на Re: logical decoding and replication of sequences  (Tomas Vondra <tomas.vondra@enterprisedb.com>)
Ответы Re: logical decoding and replication of sequences  (Tomas Vondra <tomas.vondra@enterprisedb.com>)
Список pgsql-hackers
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)



В списке pgsql-hackers по дате отправления:

Предыдущее
От: "David G. Johnston"
Дата:
Сообщение: Re: Privilege required for IF EXISTS event if the object already exists
Следующее
От: John Naylor
Дата:
Сообщение: Re: speed up text_position() for utf-8