On Mon, Jan 24, 2022 at 4:53 PM Peter Smith <smithpb2...@gmail.com> wrote: > > On Fri, Jan 21, 2022 at 2:04 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > On Thu, Jan 20, 2022 at 7:56 PM Alvaro Herrera <alvhe...@alvh.no-ip.org> > > wrote: > > > > > > > > Maybe this was meant to be "validate RF > > > > > expressions" and return, perhaps, a bitmapset of all invalid columns > > > > > referenced? > > > > > > > > Currently, we stop as soon as we find the first invalid column. > > > > > > That seems quite strange. (And above you say "gather as much info as > > > possible", so why stop at the first one?) > > > > > > > Because that is an error case, so, there doesn't seem to be any > > benefit in proceeding further. However, we can build all the required > > information by processing all publications (aka gather all > > information) and then later give an error if that idea appeals to you > > more. > > > > > > > (What is an invalid column in the first place?) > > > > > > > > A column that is referenced in the row filter but is not part of > > > > Replica Identity. > > > > > > I do wonder how do these invalid columns reach the table definition in > > > the first place. Shouldn't these be detected at DDL time and prohibited > > > from getting into the definition? > > > > > > > As mentioned by Peter E [1], there are two ways to deal with this: (a) > > The current approach is that the user can set the replica identity > > freely, and we decide later based on that what we can replicate (e.g., > > no updates). If we follow the same approach for this patch, we don't > > restrict what columns are part of the row filter, but we check what > > actions we can replicate based on the row filter. This is what is > > currently followed in the patch. (b) Add restrictions during DDL which > > is not as straightforward as it looks. > > FYI - I also wanted to highlight that doing the replica identity > validation at update/delete time is not only following the "current > approach", as mentioned above, but this is also consistent with the > *documented* behaviour in PG docs (See [1] since PG v10), > > <QUOTE> > If a table without a replica identity is added to a publication that > replicates UPDATE or DELETE operations then subsequent UPDATE or > DELETE operations will cause an error on the publisher. > </QUOTE> > > Specifically, > > It does *not* say that the RI validation error will happen when a > table is added to the publication at CREATE/ALTER PUBLICATION time. > > It says that *subsequent* "UPDATE or DELETE operations will cause an error". > > ~~ > > The point is that it is one thing to decide to change something that > was never officially documented, but to change already *documented* > behaviour is much more radical and has the potential to upset some > users. > > ------
(Sorry, fixed the broken link of the previous post) [1] https://www.postgresql.org/docs/current/logical-replication-publication.html ------ Kind Regards, Peter Smith. Fujitsu Australia
