Re: Support logical replication of DDLs

On Sun, Jun 26, 2022 at 11:47 PM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
>
> On 2022-Jun-22, vignesh C wrote:
>
> > 1) Creation of temporary table fails infinitely in the subscriber.
> > CREATE TEMPORARY TABLE temp1 (a int primary key);
> >
> > The above statement is converted to the below format:
> > CREATE TEMPORARY TABLE pg_temp.temp1 (a pg_catalog.int4 ,
> > CONSTRAINT temp1_pkey PRIMARY KEY (a));
> > While handling the creation of temporary table in the worker, the
> > worker fails continuously with the following error:
> > 2022-06-22 14:24:01.317 IST [240872] ERROR: schema "pg_temp" does not exist
>
> Perhaps one possible fix is to change the JSON format string used in
> deparse_CreateStmt. Currently, the following is used:
>
> + if (node->ofTypename)
> + fmtstr = "CREATE %{persistence}s TABLE %{if_not_exists}s %{identity}D "
> + "OF %{of_type}T %{table_elements}s "
> + "%{with_clause}s %{on_commit}s %{tablespace}s";
> + else
> + fmtstr = "CREATE %{persistence}s TABLE %{if_not_exists}s %{identity}D "
> + "(%{table_elements:, }s) %{inherits}s "
> + "%{with_clause}s %{on_commit}s %{tablespace}s";
> +
> + createStmt =
> + new_objtree_VA(fmtstr, 1,
> + "persistence", ObjTypeString,
> + get_persistence_str(relation->rd_rel->relpersistence));
>
> (Note that the word for the "persistence" element here comes straight
> from relation->rd_rel->relpersistence.) Maybe it would be more
> appropriate to set the schema to empty when the table is temp, since the
> temporary-ness is in the %{persistence} element, and thus there is no
> need to schema-qualify the table name.
>
>
> However, that would still replicate a command that involves a temporary
> table, which perhaps should not be considered fit for replication. So
> another school of thought is that if the %{persistence} is set to
> TEMPORARY, then it would be better to skip replicating the command
> altogether.
>

+1. I think it doesn't make sense to replicate temporary tables.
Similarly, we don't need to replicate the unlogged tables.

> I'm not sure how to plug that in the replication layer,
> however.
>

I see two possibilities (a) We can check the persistence and skip
logging it in the event trigger where the patch deparses the DDL and
WAL log it, or (b) We can add a similar check in pgoutput.c where we
send the DDL to downstream.

I feel (a) is better unless it is difficult to detect at that stage as
that saves additional WAL.

--
With Regards,
Amit Kapila.