Horton

Re: Support logical replication of DDLs

vignesh C <vignesh21@gmail.com>

View thread
From: vignesh C <vignesh21@gmail.com>
To: li jie <ggysxcq@gmail.com>
Cc: Ajin Cherian <itsajin@gmail.com>, rajesh singarapu <rajesh.rs0541@gmail.com>, Zheng Li <zhengli10@gmail.com>, Peter Smith <smithpb2250@gmail.com>, Dilip Kumar <dilipbalaut@gmail.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>, Amit Kapila <amit.kapila16@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, Japin Li <japinli@hotmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2022-12-06T18:52:29Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Add a run_as_owner option to subscriptions.

  2. Refactor pgoutput_change().

  3. Print the correct aliases for DML target tables in ruleutils.

  4. Fix object identity string for transforms

  5. Add grantable MAINTAIN privilege and pg_maintain role.

  6. Get rid of recursion-marker values in enum AlterTableType

  7. Release cache tuple when no longer needed

  8. Empty search_path in logical replication apply worker and walsender.

  9. Refactor format_type APIs to be more modular

  10. Use wrappers of PG_DETOAST_DATUM_PACKED() more.

Attachments

On Tue, 29 Nov 2022 at 17:51, li jie <ggysxcq@gmail.com> wrote:
>
> I will continue to give feedback for this patch.
>
> 1.  LIKE STORAGE
> ```
> CREATE TABLE ctlt (a text, c text);
> ALTER TABLE ctlt ALTER COLUMN c SET STORAGE EXTERNAL;
> CREATE TABLE ctlt_storage (LIKE ctlt INCLUDING STORAGE);
> ```
>
> postgres=# \d+ ctlt_storage
>
>                                      Table "public.ctlt_storage"
>
>  Column | Type | Collation | Nullable | Default | Storage  |
> Compression | Stats target | Description
>
> --------+------+-----------+----------+---------+----------+-------------+--------------+-------------
>
>  a      | text |           |          |         | extended |
>   |              |
>
>  c      | text |           |          |         | extended |
>   |              |
>
>
> It can be seen that the storage attribute in column C of table
> ctlt_storage is not replicated.
>
> After the CREATE TABLE LIKE statement is converted,
> the LIKE STORAGE attribute is lost because it is difficult to display
> it in the CREATE TABLE syntax.
> Maybe we need to add a statement to it, like 'ALTER TABLE ctlt_storage
> ALTER COLUMN c SET STORAGE EXTERNAL;'.
>
> 2. Reference subcommand be dropped.
> ```
> create table another (f1 int, f2 text, f3 text);
>
> alter table another
>   alter f1 type text using f2 || ' and ' || f3 || ' more',
>   alter f2 type bigint using f1 * 10,
>   drop column f3;
> ```
>
> The following error occurs downstream:
> ERROR:  column "?dropped?column?" does not exist at character 206
> STATEMENT:  ALTER TABLE public.another DROP COLUMN f3 , ALTER COLUMN
> f1 SET DATA TYPE pg_catalog.text COLLATE pg_catalog."default" USING
> (((f2 OPERATOR(pg_catalog.||) ' and '::pg_catalog.text)
> OPERATOR(pg_catalog.||) "?dropped?column?") OPERATOR(pg_catalog.||) '
> more'::pg_catalog.text), ALTER COLUMN f2 SET DATA TYPE pg_catalog.int8
> USING (f1 OPERATOR(pg_catalog.*) 10)
>
> Obviously, column f3 has been deleted and its name no longer exists.
> Maybe we need to keep it and save it in advance like a drop object.
>  However,  ATLER TABLE is complex, and this problem also occurs in
> other similar scenarios.

Currently the event trigger is called after the execution of the
command, i.e. after the drop column f3, since the column is dropped we
cannot evaluate the expression using pg_get_expr for a dropped column.
I fixed this issue
by adding an event trigger before preparation of the ALTER TYPE sub
command and the event trigger will collect the expression before the
actual execution and deparse the command later after execution. Since
the evaluated expression is stored already, there will be no need to
get the expression after execution of alter sub commands.
The attached v44 version patch has the changes for the same. Thoughts?

Regards,
Vignesh