Re: [Bug][patch]: After dropping the last label from a property graph element, invoking pg_get_propgraphdef() triggers an assertion failure
Peter Eisentraut <peter@eisentraut.org>
From: Peter Eisentraut <peter@eisentraut.org>
To: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Cc: SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com>,
PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2026-07-03T14:45:54Z
Lists: pgsql-hackers
On 19.06.26 14:05, Ashutosh Bapat wrote: > On Tue, May 12, 2026 at 12:31 PM Ashutosh Bapat > <ashutosh.bapat.oss@gmail.com> wrote: >> >> On Tue, May 12, 2026 at 7:05 AM Ashutosh Bapat >> <ashutosh.bapat.oss@gmail.com> wrote: >>> >>> On Mon, May 4, 2026 at 8:16 PM Peter Eisentraut <peter@eisentraut.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.
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Prevent dropping the last label from a property graph element
- 7afa11feca6c master landed
- ac0ad6a7c9d1 19 (unreleased) landed