Thread

1. 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> — 2026-05-04T14:46:21Z

   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.
   
   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.
   
   
   
   
   

2. 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> — 2026-05-04T14:47:51Z

   On 29.04.26 14:58, Chao Li wrote:
   > As Alvaro commented at [1], I added 0003 to use OidIsValid macro. 0001 and 0002 are unchanged.
   > 
   > [1]https://www.postgresql.org/message-id/02fe13db-4fba-4e9d-9b4c- 
   > e6271a133502@app.fastmail.com
   
   I'm personally not on board with that coding style rule.  At least I 
   don't consider this patch necessary right now as we're working toward 
   stabilizing the current code.
   
   
   
   
   

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

   Chao Li <li.evan.chao@gmail.com> — 2026-05-05T03:01:27Z

   
   > On May 4, 2026, at 22:47, Peter Eisentraut <peter@eisentraut.org> wrote:
   > 
   > On 29.04.26 14:58, Chao Li wrote:
   >> As Alvaro commented at [1], I added 0003 to use OidIsValid macro. 0001 and 0002 are unchanged.
   >> [1]https://www.postgresql.org/message-id/02fe13db-4fba-4e9d-9b4c- e6271a133502@app.fastmail.com
   > 
   > I'm personally not on board with that coding style rule.  At least I don't consider this patch necessary right now as we're working toward stabilizing the current code.
   > 
   
   In the current code in propgraphcmds.c, the style is already inconsistent. I see three forms:
   
   At line 434:
   ```
               if (OidIsValid(opc))
   ```
   
   At line 754:
   ```
       if (!labeloid)
   ```
   
   At lne 1302:
   ```
       if (pgrelid == InvalidOid)
   ```
   
   Anyway, I don’t have a strong opinion here, so please feel free to discard 0003.
   
   Best regards,
   --
   Chao Li (Evan)
   HighGo Software Co., Ltd.
   https://www.highgo.com/
   
   
   
   
   
   
   
   

4. 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> — 2026-05-12T01:35:06Z

   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?
   
   -- 
   Best Wishes,
   Ashutosh Bapat
   
   
   
   

5. 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> — 2026-05-12T07:01:03Z

   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