Re: Column Filtering in Logical Replication

Поиск
Список
Период
Сортировка
От Justin Pryzby
Тема Re: Column Filtering in Logical Replication
Дата
Msg-id 20211230221634.GT24477@telsasoft.com
обсуждение исходный текст
Ответ на Re: Column Filtering in Logical Replication  (Alvaro Herrera <alvherre@alvh.no-ip.org>)
Ответы Re: Column Filtering in Logical Replication  (Alvaro Herrera <alvherre@alvh.no-ip.org>)
Список pgsql-hackers
> +    bool        am_partition = false;
>...
>    Assert(!isnull);
>    lrel->relkind = DatumGetChar(slot_getattr(slot, 3, &isnull));
>    Assert(!isnull);
> +    am_partition = DatumGetChar(slot_getattr(slot, 4, &isnull));

I think this needs to be GetBool.
You should Assert(!isnull) like the others.
Also, I think it doesn't need to be initialized to "false".

> +        /*
> +         * Even if the user listed all columns in the column list, we cannot
> +         * allow a column list to be specified when REPLICA IDENTITY is FULL;
> +         * that would cause problems if a new column is added later, because
> +         * that could would have to be included (because of being part of the

could would is wrong

> +    /*
> +     * Translate list of columns to attnums. We prohibit system attributes and
> +     * make sure there are no duplicate columns.
> +     *
> +     */

extraneous line

> +/*
> + * Gets a list of OIDs of all column-partial publications of the given
> + * relation, that is, those that specify a column list.

I would call this a "partial-column" publication.

> +                    errmsg("cannot set REPLICA IDENTITY FULL when column-partial publications exist"));
> +     * Check column-partial publications.  All publications have to include all

same

> +    /*
> +     * Store the column names only if they are contained in column filter

period(.)

> +     * LogicalRepRelation will only contain attributes corresponding to those
> +     * specficied in column filters.

specified

> --- a/src/include/catalog/pg_publication_rel.h
> +++ b/src/include/catalog/pg_publication_rel.h
> @@ -31,6 +31,9 @@ CATALOG(pg_publication_rel,6106,PublicationRelRelationId)
>      Oid            oid;            /* oid */
>      Oid            prpubid BKI_LOOKUP(pg_publication); /* Oid of the publication */
>      Oid            prrelid BKI_LOOKUP(pg_class);    /* Oid of the relation */
> +#ifdef CATALOG_VARLEN
> +    int2vector    prattrs;        /* Variable length field starts here */
> +#endif

The language in the pre-existing comments is better:
    /* variable-length fields start here */

> @@ -791,12 +875,13 @@ fetch_remote_table_info(char *nspname, char *relname,
>
>                 ExecClearTuple(slot);
>         }
> +
>         ExecDropSingleTupleTableSlot(slot);
> +       walrcv_clear_result(res);
> +       pfree(cmd.data);
>
>         lrel->natts = natt;
>
> -       walrcv_clear_result(res);
> -       pfree(cmd.data);
>  }

The blank line after "lrel->natts = natt;" should be removed.



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

Предыдущее
От: Andrew Dunstan
Дата:
Сообщение: Re: Why is src/test/modules/committs/t/002_standby.pl flaky?
Следующее
От: Tom Lane
Дата:
Сообщение: More pg_dump performance hacking