Re: [Bug][patch]: After dropping the last label from a property graph element, invoking pg_get_propgraphdef() triggers an assertion failure

On Fri, Jul 3, 2026 at 8:15 PM Peter Eisentraut <peter(at)eisentraut(dot)org> wrote:
>
> On 19.06.26 14:05, Ashutosh Bapat wrote:
> > On Tue, May 12, 2026 at 12:31 PM Ashutosh Bapat
> > <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
> >>
> >> On Tue, May 12, 2026 at 7:05 AM Ashutosh Bapat
> >> <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
> >>>
> >>> On Mon, May 4, 2026 at 8:16 PM Peter Eisentraut <peter(at)eisentraut(dot)org> 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/E3D6552E-DB26-47BC-9C7C-0F9CB936BF5B@gmail.com
>
> 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