Re: Support logical replication of DDLs

On Thu, Jun 2, 2022 at 5:44 PM houzj.fnst@fujitsu.com
<houzj.fnst@fujitsu.com> wrote:
>
> The main idea of replicating the CREATE TABLE AS is that we deprase the CREATE
> TABLE AS into a simple CREATE TABLE(without subquery) command and WAL log it
> after creating the table and before writing data into the table and replicate
> the incoming writes later as normal INSERTs. In this apporach, we don't execute
> the subquery on subscriber so that don't need to make sure all the objects
> referenced in the subquery also exists in subscriber. And This approach works
> for all kind of commands(e.g. CRAETE TABLE AS [SELECT][EXECUTE][VALUES])
>
> One problem of this approach is that we cannot use the current trigger to
> deparse or WAL log the CREATE TABLE. Because none of the even trigger is fired
> after creating the table and before inserting the data. To solve this, one idea
> is that we could directly add some code at the end of create_ctas_internal() to
> deparse and WAL log it. Moreover, we could even introduce a new type of event
> trigger(table_create) which would be fired at the expected timing so that we
> can use the trigger function to deparse and WAL log. I am not sure which way is
> better.
>

I am also not able to think of a better way to replicate CREATE TABLE
AS/SELECT INTO other than to use a new type of event trigger. I think
it is better to discuss this new type of event trigger in a separate
thread with the use case of DDL replication unless we have a better
idea to achieve this.

Few comments:
===============
1. Why not capture the CREATE TABLE/CREATE TABLE AS ... command with
EventTriggerCollectSimpleCommand instead of using
EventTriggerCreateTableStart?

2.
+ /*
+ * Use PG_TRY to ensure parsetree is reset even when one trigger
+ * fails. (This is perhaps not necessary, as the currentState variable will
+ * be removed shortly by our caller, but it seems better to play safe.)
+ */
+ old_parsetree = currentEventTriggerState->currentCommand->parsetree;
+ currentEventTriggerState->currentCommand->d.simple.address = address;
+ currentEventTriggerState->currentCommand->parsetree = parsetree;

Instead of doing this can't we use the parsetree stored in trigdata to
deparse the statement?

3. Is there a reason to invoke the trigger after defining the relation
instead of doing it before similar to table_rewrite trigger
(EventTriggerTableRewrite).

4. It should be published for all tables publication similar to 'create table'

5. The new code in CreatePublication looks quite haphazard, can we
improve it by moving it into separate functions?

-- 
With Regards,
Amit Kapila.