On Mon, May 4, 2026 at 8:16 PM Peter Eisentraut <[email protected]> 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


Reply via email to