On 19.06.26 14:05, Ashutosh Bapat wrote:
On Tue, May 12, 2026 at 12:31 PM Ashutosh Bapat
<[email protected]> wrote:
On Tue, May 12, 2026 at 7:05 AM Ashutosh Bapat
<[email protected]> wrote:
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?
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/[email protected]
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.
