Re: [PATCH] Logical decoding of TRUNCATE

Hi,

I reviewed 0001 in its own thread.

So I think that we generally want this patch and I think the design
decisions are right. Namely:

TRUNCATE being treated as DELETE in terms of DML filtering makes sense
to me as it is basically bulk delete, needs to be mentioned in release
notes though.

Adding special message to protocol is appropriate as truncate is more
DML than DDL in sense of manipulating data so it should be replicated
separately from other DDL

Processing relations that were truncated when CASCADE is used separately
is needed because we allow relations to be filtered by logical replication

I see the patch adds new xlog record which is perhaps not ideal but the
current one seems utterly unsuitable for decoding so I guess it's okay,
especially when it's only added for wal_level = logical which it is.
Also TRUNCATE is not exactly high tps operation.

Things I am less convinced about:

The patch will cascade truncation on downstream if cascade was specified
on the upstream, that can potentially be dangerous and we either should
not do it and only truncate the tables which were truncated upstream
(but without restricting because of FKs), leaving the data inconsistent
on downstream (like we do already with DELETE or UPDATE). Or maybe make
it into either subscription or publication option so that user can chose
the behaviour here as I am sure some people will want it to cascade (but
the default should still IMHO be to not cascade as that's safer).

> + /* logicalrep_rel_close call not needed, because ExecuteTruncateGuts
> + * already closes the relations. Setting localrel to NULL in the map entry
> + * is still needed.
> + */
> + rel->localrel = NULL;

This is somewhat ugly. Perhaps the ExecuteTruncateGuts should track
which relations it opened and only close those and the rest should be
closed by caller? That should also remove the other ugly part which is
that the ExecuteTruncateGuts modifies the input list. What if caller
wanted to use those relations it sent as parameter later?

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services