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

From: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>
To: Peter Eisentraut <peter(at)eisentraut(dot)org>
Cc: SATYANARAYANA NARLAPURAM <satyanarlapuram(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: [Bug][patch]: After dropping the last label from a property graph element, invoking pg_get_propgraphdef() triggers an assertion failure
Date: 2026-06-19 12:05:32
Message-ID: CAExHW5teYi1Kyw2exqk8t+WF4wq0cUaWcWQo0oi7NpnmcctJHA@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

--
Best Wishes,
Ashutosh Bapat

Attachment Content-Type Size
v20260619-0003-Dropping-a-property-not-associated-with-th.patch text/x-patch 6.3 KB
v20260619-0002-Test-concurrent-modifications-to-a-propert.patch text/x-patch 9.9 KB
v20260619-0001-Prevent-dropping-the-last-label-from-a-pro.patch text/x-patch 7.1 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Henson Choi 2026-06-19 12:08:19 Re: Row pattern recognition
Previous Message Michael Paquier 2026-06-19 11:12:47 Re: Unexpected behavior after OOM errors