On Thu, Jun 16, 2022 at 12:30 PM Amit Langote <amitlangot...@gmail.com> wrote: > > On Thu, Jun 16, 2022 at 3:45 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Thu, Jun 16, 2022 at 11:43 AM Amit Langote <amitlangot...@gmail.com> > > wrote: > > > + * Don't throw any error here just mark the relation entry as not > > > updatable, > > > + * as replica identity is only for updates and deletes but inserts can be > > > + * replicated even without it. > > > > > > I know you're simply copying the old comment, but I think we can > > > rewrite it to be slightly more useful: > > > > > > We just mark the relation entry as not updatable here if the local > > > replica identity is found to be insufficient and leave it to > > > check_relation_updatable() to throw the actual error if needed. > > > > I am fine with improving this comment but it would be better if in > > some way we keep the following part of the comment: "as replica > > identity is only for updates and deletes but inserts can be replicated > > even without it." as that makes it more clear why it is okay to just > > mark the entry as not updatable. One idea could be: "We just mark the > > relation entry as not updatable here if the local replica identity is > > found to be insufficient and leave it to check_relation_updatable() to > > throw the actual error if needed. This is because replica identity is > > only for updates and deletes but inserts can be replicated even > > without it.". Feel free to suggest if you have any better ideas? > > I thought mentioning check_relation_updatable() would make it clear > that only updates (and deletes) care about a valid local replica > identity, because only apply_handle_{update|delete}() call that > function. Anyway, how about this: > > We just mark the relation entry as not updatable here if the local > replica identity is found to be insufficient for applying > updates/deletes (inserts don't care!) and leave it to > check_relation_updatable() to throw the actual error if needed. >
This sounds better to me than the previous text. -- With Regards, Amit Kapila.