On 19/01/18 12:37, Marco Nenciarini wrote: > Hi All, > > Il 18/01/18 17:48, Simon Riggs ha scritto: >> On 17 January 2018 at 17:07, Petr Jelinek <petr.jeli...@2ndquadrant.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. >> > > I agree that not replicating the CASCADE option is the best option > according to POLA principle. > >>>> + /* 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 >> > > Attached a new version of the patch addressing these issues. >
Besides the small thing I wrote for the 0001 in the other thread I am pretty much happy with this now. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services