Re: [PATCH] Logical decoding of TRUNCATE

On 17 January 2018 at 17:07, Petr Jelinek <petr(dot)jelinek(at)2ndquadrant(dot)com> wrote:

> 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).

I agree the default should be to NOT cascade.

If someone wants cascading as a publication option, that can be added later.

>> + /* 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?

Agreed

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services