Re: [Bug][patch]: After dropping the last label from a property graph element, invoking pg_get_propgraphdef() triggers an assertion failure
Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
From: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
To: Peter Eisentraut <peter@eisentraut.org>
Cc: SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2026-05-12T07:01:03Z
Lists: pgsql-hackers
Attachments
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. -- Best Wishes, Ashutosh Bapat