On 4/29/22 06:48, Amit Kapila wrote: > On Thu, Apr 28, 2022 at 11:00 PM Tomas Vondra > <tomas.von...@enterprisedb.com> wrote: >> >> On 4/28/22 14:26, Peter Eisentraut wrote: >>> On 27.04.22 12:33, Amit Kapila wrote: >>> >>> I wonder how we handle the combination of >>> >>> pub1: publish=insert WHERE (foo) >>> pub2: publish=update WHERE (bar) >>> >>> I think it would be incorrect if the combination is >>> >>> pub1, pub2: publish=insert,update WHERE (foo OR bar). >> >> That's a good question, actually. No, we don't combine the publications >> like this, the row filters are kept "per action". >> > > Right, and it won't work even if try to combine in this case because > of replica identity restrictions. > >> But the exact behavior >> turns out to be rather confusing in this case. >> >> (Note: This has nothing to do with column lists.) >> >> Consider an example similar to what Alvaro posted earlier: >> >> create table uno (a int primary key, b int, c int); >> >> create publication uno for table uno where (a > 0) >> with (publish='insert'); >> >> create publication dos for table uno where (a < 0) >> with (publish='update'); >> >> And do this: >> >> insert into uno values (1, 2, 3), (-1, 3, 4) >> >> which on the subscriber produces just one row, because (a<0) replicates >> only updates: >> >> a | b | c >> ---+---+--- >> 1 | 2 | 3 >> (1 row) >> >> Now, let's update the (a<0) row. >> >> update uno set a = 2 where a = -1; >> >> It might seem reasonable to expect the updated row (2,3,4) to appear on >> the subscriber, but no - that's not what happens. Because we have (a<0) >> for UPDATE, and we evaluate this on the old row (matches) and new row >> (does not match). And pgoutput_row_filter() decides the update needs to >> be converted to DELETE, despite the old row was not replicated at all. >> > > Right, but we don't know what previously would have happened maybe the > user would have altered the publication action after the initial row > is published in which case this DELETE is required as is shown in the > example below. We can only make the decision based on the current > tuple. For example: > > create table uno (a int primary key, b int, c int); > > create publication uno for table uno where (a > 0) > with (publish='insert'); > > create publication dos for table uno where (a < 0) > with (publish='insert'); > > -- create subscription for both these publications. > > insert into uno values (1, 2, 3), (-1, 3, 4); > > Alter publication dos set (publish='update'); > > update uno set a = 2 where a = -1; > > Now, in this case, the old row was replicated and we would need a > DELETE corresponding to it. >
I think such issues due to ALTER of the publication are somewhat expected, and I think users will understand they might need to resync the subscription or something like that. A similar example might be just changing the where condition, create publication p for table t where (a > 10); and then alter publication p set table t where (a > 15); If we replicated any rows with (a > 10) and (a <= 15), we'll just stop replicating them. But if we re-create the subscription, we end up with a different set of rows on the subscriber, omitting rows with (a <= 15). In principle we'd need to replicate the ALTER somehow, to delete or insert the rows that start/stop matching the row filter. It's a bit similar to not replicating DDL, perhaps. But I think the issue I've described is different, because you don't have to change the subscriptions at all and you'll still have the problem. I mean, imagine doing this: -- publisher create table t (a int primary key, b int); create publication p for table t where (a > 10) with (publish='update'); -- subscriber create table t (a int primary key, b int); create subscription s connection '...' publication p; -- publisher insert into t select i, i from generate_series(1,20) s(i); update t set b = b * 10; -- subscriber --> has no rows in "t" --> recreate the subscription drop subscription s; create subscription s connection '...' publication p; --> now it has all the rows with (a>10), because tablesync ignores publication actions The reason why I find this really annoying is that it makes it almost impossible to setup two logical replicas that'd be "consistent", unless you create them at the same time (= without any writes in between). And it's damn difficult to think about the inconsistencies. IMHO this all stems from allowing row filters and restricting pubactions at the same time (notice this only used a single publication). So maybe the best option would be to disallow combining these two features? That would ensure the row filter filter is always applied to all actions in a consistent manner, preventing all these issues. Maybe that's not possible - maybe there are valid use cases that would need such combination, and you mentioned replica identity might be an issue (and maybe requiring RIF with row filters is not desirable). So maybe we should at least warn against this in the documentation? >> I'm not sure if pgoutput_row_filter() can even make reasonable decisions >> with such configuration (combination of row filters, actions ...). But >> it sure seems confusing, because if you just inserted the updated row, >> it would get replicated. >> > > True, but that is what the combination of publications suggests. The > publication that publishes inserts have different criteria than > updates, so such behavior (a particular row when inserted will be > replicated but when it came as a result of an update it won't be > replicated) is expected. > >> Which brings me to a second problem, related to this one. Imagine you >> create the subscription *after* inserting the two rows. In that case you >> get this: >> >> a | b | c >> ----+---+--- >> 1 | 2 | 3 >> -1 | 3 | 4 >> (2 rows) >> >> because tablesync.c ignores which actions is the publication (and thus >> the rowfilter) defined for. >> > > Yeah, this is the behavior of tablesync.c with or without rowfilter. > It ignores publication actions. So, if you update any tuple before > creation of subscription it will be replicated but the same update > won't be replicated after initial sync if the publication just > publishes 'insert'. I think we can't decide which data to copy based > on publication actions as COPY wouldn't know if a particular row is > due to a fresh insert or due to an update. In your example, it is > possible that row (-1, 3, 4) would have been there due to an update. > Right. Which is why I think disallowing these two features (filtering actions and row filters) might prevent this, because it eliminates this ambiguity. It would not matter if a row was INSERTed or UPDATEd when evaluating the row filter. > >> I think it's natural to expect that (INSERT + sync) and (sync + INSERT) >> produce the same output on the subscriber. >> >> >> I'm not sure we can actually make this perfectly sane with arbitrary >> combinations of filters and actions. It would probably depend on whether >> the actions are commutative, associative and stuff like that. But maybe >> we can come up with restrictions that'd make this sane? >> > > True, I think to some extent we rely on users to define it sanely > otherwise currently also it can easily lead to even replication being > stuck. This can happen when the user is trying to operate on the same > table and define publication/subscription on multiple nodes for it. > See [1] where we trying to deal with such a problem. > > [1] - https://commitfest.postgresql.org/38/3610/ > That seems to deal with a circular replication, i.e. two logical replication links - a bit like a multi-master. Not sure how is that related to the issue we're discussing here? regards -- Tomas Vondra EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
