On Fri, Sep 17, 2021 at 7:38 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > On Fri, Sep 17, 2021 at 11:36 AM houzj.f...@fujitsu.com > <houzj.f...@fujitsu.com> wrote: > > On Thursday, September 16, 2021 6:05 PM Amit Kapila > > <amit.kapil...@gmail.com> > > > > On Tuesday, September 14, 2021 10:41 PM vignesh C <vignes...@gmail.com> > > > > wrote: > > > > > On Tue, Sep 7, 2021 at 11:38 AM houzj.f...@fujitsu.com > > > > > <houzj.f...@fujitsu.com> wrote: > > > > > > > > Thanks for the comment. > > > > Attach new version patches which clean the table at the end. > > > > > > > > > > + * For partitioned table contained in the publication, we must > > > + * invalidate all partitions contained in the respective partition > > > + * trees, not just the one explicitly mentioned in the publication. > > > > > > Can we slightly change the above comment as: "For the partitioned tables, > > > we > > > must invalidate all partitions contained in the respective partition > > > hierarchies, > > > not just the one explicitly mentioned in the publication. This is required > > > because we implicitly publish the child tables when the parent table is > > > published." > > > > > > Apart from this, the patch looks good to me. > > > > > > I think we need to back-patch this till v13. What do you think? > > > > I agreed. > > > > Attach patches for back-branch, each has passed regression tests and > > pgindent. > > Thanks, your patches look good to me. I'll push them sometime next > week after Tuesday unless there are any comments.
Thanks Amit, Tang, and Hou for this. Sorry that I didn't comment on this earlier, but I think either GetPubPartitionOptionRelations() or InvalidatePublicationRels() introduced in the commit 4548c76738b should lock the partitions, just like to the parent partitioned table would be, before invalidating them. There may be some hazards to invalidating a relation without locking it. For example, maybe add a 'lockmode' parameter to GetPubPartitionOptionRelations() which it passes down to find_all_inheritors() instead of NoLock as now. And make all sites except GetPublicationRelations() pass ShareUpdateExclusiveLock for it. Maybe like the attached. -- Amit Langote EDB: http://www.enterprisedb.com
lock-publication-partitions-before-inval.patch
Description: Binary data
