Re: [PATCH] Logical decoding of TRUNCATE

On Mon, Dec 21, 2020 at 09:42:47AM -0800, Andres Freund wrote: > On 2020-12-20 15:54:31 -0800, Peter Geoghegan wrote: > > On Sun, Dec 20, 2020 at 3:13 PM Andres Freund wrote: > > > Hm. Do I understand correctly that this problem is hit solely because > > > the parallel mode code relies on there

Re: [PATCH] Logical decoding of TRUNCATE

Hi, On 2020-12-20 15:54:31 -0800, Peter Geoghegan wrote: > On Sun, Dec 20, 2020 at 3:13 PM Andres Freund wrote: > > Hm. Do I understand correctly that this problem is hit solely because > > the parallel mode code relies on there already have been a transaction > > snapshot set, thus avoiding the

Re: [PATCH] Logical decoding of TRUNCATE

On Sun, Dec 20, 2020 at 03:54:31PM -0800, Peter Geoghegan wrote: > On Sun, Dec 20, 2020 at 3:13 PM Andres Freund wrote: > > Hm. Do I understand correctly that this problem is hit solely because > > the parallel mode code relies on there already have been a transaction > > snapshot set, thus

Re: [PATCH] Logical decoding of TRUNCATE

On Sun, Dec 20, 2020 at 9:43 AM Noah Misch wrote: > > Since commit 039eb6e added logical replication support for TRUNCATE, logical > apply of the TRUNCATE fails if it chooses a parallel index build: > I think the TRUNCATE operation should not use parallelism either via apply worker or without it

Re: [PATCH] Logical decoding of TRUNCATE

On Sun, Dec 20, 2020 at 3:13 PM Andres Freund wrote: > Hm. Do I understand correctly that this problem is hit solely because > the parallel mode code relies on there already have been a transaction > snapshot set, thus avoiding the error? And that the code normally only > works because

Re: [PATCH] Logical decoding of TRUNCATE

Hi, On 2020-12-20 04:13:19 +, Noah Misch wrote: > postgres: subscriber: logical replication worker for subscription 16411 > (GetTransactionSnapshot+0x168) [0x951ce8] > postgres: subscriber: logical replication worker for subscription 16411 > (InitializeParallelDSM+0x16)

Re: [PATCH] Logical decoding of TRUNCATE

On Sat, Apr 07, 2018 at 07:40:11PM -0400, Peter Eisentraut wrote: > Committed with those changes. Since commit 039eb6e added logical replication support for TRUNCATE, logical apply of the TRUNCATE fails if it chooses a parallel index build: cat >/tmp/most_parallel.conf <

Re: [PATCH] Logical decoding of TRUNCATE

On 4/5/18 13:07, Alvaro Herrera wrote: > Note that you start the loop having the Relation; yet you go extra > length to grab the relnamespace and relname from syscache instead of > just relations[i]->rd_rel->relname etc. fixed > Maybe not a big deal, but for future pg_waldump users I'm sure

Re: [PATCH] Logical decoding of TRUNCATE

This sounds like a good approach. > +static void > +pg_decode_truncate(LogicalDecodingContext *ctx, ReorderBufferTXN *txn, > +int nrelations, Relation relations[], > ReorderBufferChange *change) > +{ > + for (i = 0; i < nrelations; i++) > + { > +

Re: [PATCH] Logical decoding of TRUNCATE

On 4/3/18 22:31, Peter Eisentraut wrote: > The problem I see now is that when we WAL-log a truncate, we include all > the relids in one record. That seems useful. But during decoding we > split this into one change per relid. So at the receiving end, truncate > can only process one relation at

Re: [PATCH] Logical decoding of TRUNCATE

On 4 April 2018 at 03:31, Peter Eisentraut wrote: > I wonder why this approach was chosen. I looked at coding it that way and it seemed worse than the way chosen. > I'm going to try to hack up an alternative approach and see how it works > out. If you add a

Re: [PATCH] Logical decoding of TRUNCATE

Here is an updated patch that addresses some of your concerns. I've split it up into the decoding support and the pgoutput replication support. The problem I see now is that when we WAL-log a truncate, we include all the relids in one record. That seems useful. But during decoding we split

Re: [PATCH] Logical decoding of TRUNCATE

> On Apr 2, 2018, at 11:50, Andres Freund wrote: > I'm not convinced. I think it's perfectly reasonable to want to truncate > away data on the live node, but maintain it on an archival node. +1. We've had at least one specific request for this, in maintaining a data

Re: [PATCH] Logical decoding of TRUNCATE

Hi, On 2018-04-02 07:39:57 +0100, Simon Riggs wrote: > On 1 April 2018 at 21:01, Andres Freund wrote: > > >> *** > >> *** 111,116 CREATE PUBLICATION >> class="parameter">name > >> --- 111,121 > >> and so the default value for this option is

Re: [PATCH] Logical decoding of TRUNCATE

Hi, On 2018-04-02 14:30:50 -0400, Peter Eisentraut wrote: > On 4/1/18 16:01, Andres Freund wrote: > > On 2018-01-25 14:21:15 +0100, Marco Nenciarini wrote: > >> + if (SessionReplicationRole != SESSION_REPLICATION_ROLE_REPLICA) > >> + { > >> + > >> + /* > >> + * Check foreign

Re: [PATCH] Logical decoding of TRUNCATE

On 2 April 2018 at 19:30, Peter Eisentraut wrote: >>> *** >>> *** 111,116 CREATE PUBLICATION >> class="parameter">name >>> --- 111,121 >>> and so the default value for this option is >>> 'insert, update, delete'. >>>

Re: [PATCH] Logical decoding of TRUNCATE

On 4/1/18 16:01, Andres Freund wrote: > On 2018-01-25 14:21:15 +0100, Marco Nenciarini wrote: >> +if (SessionReplicationRole != SESSION_REPLICATION_ROLE_REPLICA) >> +{ >> + >> +/* >> + * Check foreign key references. In CASCADE mode, this should >> be >> +

Re: [PATCH] Logical decoding of TRUNCATE

On 1 April 2018 at 21:01, Andres Freund wrote: >> *** >> *** 111,116 CREATE PUBLICATION > class="parameter">name >> --- 111,121 >> and so the default value for this option is >> 'insert, update, delete'. >> >> +

Re: [PATCH] Logical decoding of TRUNCATE

On 2018-01-25 14:21:15 +0100, Marco Nenciarini wrote: > + if (SessionReplicationRole != SESSION_REPLICATION_ROLE_REPLICA) > + { > + > + /* > + * Check foreign key references. In CASCADE mode, this should > be > + * unnecessary since we just pulled

Re: [PATCH] Logical decoding of TRUNCATE

On 26/01/18 03:44, Peter Eisentraut wrote: > On 1/24/18 07:53, Petr Jelinek wrote: >> That depends on if we consider this to be part of sequence handling or >> truncate statement replication handling. It's true that if we had >> sequence replication, the restart would get propagated that way

Re: [PATCH] Logical decoding of TRUNCATE

On 1/24/18 07:53, Petr Jelinek wrote: > That depends on if we consider this to be part of sequence handling or > truncate statement replication handling. It's true that if we had > sequence replication, the restart would get propagated that way anyway. > OTOH, if we'll want to add option in the

Re: [PATCH] Logical decoding of TRUNCATE

On 26/01/18 02:34, Robert Haas wrote: > On Wed, Jan 17, 2018 at 12:07 PM, Petr Jelinek > wrote: >> 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

Re: [PATCH] Logical decoding of TRUNCATE

On Wed, Jan 17, 2018 at 12:07 PM, Petr Jelinek wrote: > 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

Re: [PATCH] Logical decoding of TRUNCATE

Il 25/01/18 13:18, Petr Jelinek ha scritto: > On 25/01/18 08:26, Thomas Munro wrote: >> On Wed, Jan 24, 2018 at 6:47 AM, Marco Nenciarini >> wrote: >>> Version 16 attached. >> >> Hi Marco, >> >> FYI this version doesn't compile: >> >> worker.c: In function

Re: [PATCH] Logical decoding of TRUNCATE

On 25/01/18 08:26, Thomas Munro wrote: > On Wed, Jan 24, 2018 at 6:47 AM, Marco Nenciarini > wrote: >> Version 16 attached. > > Hi Marco, > > FYI this version doesn't compile: > > worker.c: In function ‘apply_handle_truncate’: > worker.c:888:39: error:

Re: [PATCH] Logical decoding of TRUNCATE

I wonder whether this should be dealing with sequences at all. We are not currently publishing any information about sequences, so it seems weird to do it only here. Also, I'd imagine that if we ever get to publishing sequence events, then the sequence restarts would be published as independent

Re: [PATCH] Logical decoding of TRUNCATE

Hi, On 23/01/18 18:47, Marco Nenciarini wrote: > Il 23/01/18 18:25, Petr Jelinek ha scritto: >> On 23/01/18 18:19, Marco Nenciarini wrote: >>> Il 23/01/18 18:13, Petr Jelinek ha scritto: Hi, On 23/01/18 15:38, Marco Nenciarini wrote: > Il 22/01/18 23:18, Petr Jelinek ha

Re: [PATCH] Logical decoding of TRUNCATE

On 23/01/18 18:19, Marco Nenciarini wrote: > Il 23/01/18 18:13, Petr Jelinek ha scritto: >> Hi, >> >> On 23/01/18 15:38, Marco Nenciarini wrote: >>> Il 22/01/18 23:18, Petr Jelinek ha scritto: On 22/01/18 19:45, Petr Jelinek wrote: Actually on second look, I don't like the new

Re: [PATCH] Logical decoding of TRUNCATE

Il 23/01/18 18:13, Petr Jelinek ha scritto: > Hi, > > On 23/01/18 15:38, Marco Nenciarini wrote: >> Il 22/01/18 23:18, Petr Jelinek ha scritto: >>> On 22/01/18 19:45, Petr Jelinek wrote: >>> >>> Actually on second look, I don't like the new boolean parameter much. >>> I'd rather we didn't touch

Re: [PATCH] Logical decoding of TRUNCATE

Hi, On 23/01/18 15:38, Marco Nenciarini wrote: > Il 22/01/18 23:18, Petr Jelinek ha scritto: >> On 22/01/18 19:45, Petr Jelinek wrote: >> >> Actually on second look, I don't like the new boolean parameter much. >> I'd rather we didn't touch the input list and always close only >> relations opened

Re: [PATCH] Logical decoding of TRUNCATE

Il 22/01/18 23:18, Petr Jelinek ha scritto: > On 22/01/18 19:45, Petr Jelinek wrote: > > Actually on second look, I don't like the new boolean parameter much. > I'd rather we didn't touch the input list and always close only > relations opened inside the ExecuteTruncateGuts(). > > It may mean

Re: [PATCH] Logical decoding of TRUNCATE

On 22/01/18 19:45, Petr Jelinek wrote: > On 19/01/18 12:37, Marco Nenciarini wrote: >> Hi All, > + /* logicalrep_rel_close call not needed, because ExecuteTruncateGuts > + * already closes the relations. Setting localrel to NULL in the > map entry > + * is still

Re: [PATCH] Logical decoding of TRUNCATE

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 >> wrote: >> >>> Things I am less convinced about: >>> >>> The patch will cascade truncation on downstream if cascade

Re: [PATCH] Logical decoding of TRUNCATE

Hi All, Il 18/01/18 17:48, Simon Riggs ha scritto: > On 17 January 2018 at 17:07, Petr Jelinek > wrote: > >> Things I am less convinced about: >> >> The patch will cascade truncation on downstream if cascade was specified >> on the upstream, that can potentially

Re: [PATCH] Logical decoding of TRUNCATE

On 17 January 2018 at 17:07, Petr Jelinek 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

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.

Re: [PATCH] Logical decoding of TRUNCATE

Attached here there is the complete list of patches required to pass all the tests. The 0001 patch is discussed in a separate thread, but I've posted it also here to ease the review of the 0002. Regards, Marco -- Marco Nenciarini - 2ndQuadrant Italy PostgreSQL Training, Services and Support

Re: [PATCH] Logical decoding of TRUNCATE

Patch rebased on the current master. Regards, Marco -- Marco Nenciarini - 2ndQuadrant Italy PostgreSQL Training, Services and Support marco.nenciar...@2ndquadrant.it | www.2ndQuadrant.it diff --git a/contrib/test_decoding/expected/ddl.out b/contrib/test_decoding/expected/ddl.out index

Re: [PATCH] Logical decoding of TRUNCATE

Hi, I've found some SGML errors in the version v10 of the patch. I've fixed it in version v11 that is attached. Regards, Marco -- Marco Nenciarini - 2ndQuadrant Italy PostgreSQL Training, Services and Support marco.nenciar...@2ndquadrant.it | www.2ndQuadrant.it diff --git

Re: [PATCH] Logical decoding of TRUNCATE

On 29 December 2017 at 19:55, Andres Freund wrote: > Hi, > > On 2017-12-29 14:15:22 +0100, Marco Nenciarini wrote: >> This patch implements support for TRUNCATE statements >> in logical replication. The work has mainly done by Simon Riggs then >> finished by me. Tests are

Re: [PATCH] Logical decoding of TRUNCATE

Hi, On 2017-12-29 14:15:22 +0100, Marco Nenciarini wrote: > This patch implements support for TRUNCATE statements > in logical replication. The work has mainly done by Simon Riggs then > finished by me. Tests are written by me. > > TRUNCATE is treated as a form of DELETE for the purpose of