On Fri, Jul 3, 2026 at 8:15 PM Peter Eisentraut <[email protected]> wrote: > > On 19.06.26 14:05, Ashutosh Bapat wrote: > > On Tue, May 12, 2026 at 12:31 PM Ashutosh Bapat > > <[email protected]> wrote: > >> > >> On Tue, May 12, 2026 at 7:05 AM Ashutosh Bapat > >> <[email protected]> wrote: > >>> > >>> On Mon, May 4, 2026 at 8:16 PM Peter Eisentraut <[email protected]> > >>> wrote: > >>>> > >>>> On 28.04.26 17:02, Ashutosh Bapat wrote: > >>>>> We are looking up element label catalogs twice in this patch - first > >>>>> to find the label to be dropped and then to find the number of labels > >>>>> associated with the given element. I combined these two into a single > >>>>> while loop. > >>>> > >>>> That looks okay, but I think the names of the local variables are now a > >>>> bit off. I would expect elrel and elscan to refer to > >>>> pg_propgraph_element, not pg_propgraph_element_label. Maybe use > >>>> ellabelrel etc. > >>> > >>> Done. > >>> > >>>> > >>>> Also, I think this code needs to think a bit about locking to handle the > >>>> situation where more than one DROP LABEL operation happens concurrently. > >>>> > >>> > >>> AlterPropGraph already takes ShareRowExclusiveLock at the beginning so > >>> only one label can be dropped at a time. I have added an isolation > >>> test to test the scenario. We could further add some more tests to > >>> make sure that properties can not be added to a label being dropped, > >>> adding label to an element being dropped, adding label to an element > >>> being added etc. Would that be an overkill? > >> > >> Here's the patchset without the extra tests. > > > > I decided to go ahead and added those extra tests as a separate patch. > > They don't cover all the possible concurrent modifications, but the > > ones which may render a property or a label orphan. > > > > 0001 - patch to avoid dropping last label from an element > > 0002 - patch to test concurrent ALTER PROPERTY GRAPH > > 0003 - patch from [1] which would conflict with 0001, if not included here. > > > > [1] > > https://www.postgresql.org/message-id/[email protected] > > I have committed 0001 and 0003. You are right that the lock prevents > concurrent modifications. This is pretty straightforward, so I have > omitted the patch 0002 with the new tests. I have added a brief comment > where the lock is taken. >
Thanks. Works for me. -- Best Wishes, Ashutosh Bapat
