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

Started by SATYANARAYANA NARLAPURAM11 days ago8 messageshackers
Jump to latest
#1SATYANARAYANA NARLAPURAM
satyanarlapuram@gmail.com

Hi hackers,

ALTER PROPERTY GRAPH ... ALTER ... DROP LABEL currently allows removing
the last label from an element, leaving it with zero labels.

On assert-enabled builds, pg_get_propgraphdef() hits
TRAP: failed Assert("count > 0"), File: "ruleutils.c", Line: 1837, PID:
1821840

Repro:

CREATE TABLE t (x int PRIMARY KEY, y int, z int);
CREATE PROPERTY GRAPH g VERTEX TABLES (t KEY (x) LABEL l1 LABEL l2);
ALTER PROPERTY GRAPH g ALTER VERTEX TABLE t DROP LABEL l2;
ALTER PROPERTY GRAPH g ALTER VERTEX TABLE t DROP LABEL l1;
SELECT pg_get_propgraphdef('g'::regclass);

We can fix it two ways, (1) Prevent dropping the last label; (2) handle
zero labels.
I feel it is easier to prevent dropping the last label than handling zero
labels. Thoughts?

The attached patch adds a check in AlterPropGraph() before
performDeletion(). It scans pg_propgraph_element_label to count labels
for the element, and raises an error if only one remains. A regression test
is included
that drops labels down to the last one, verifies the error, then re-adds
them back.

Thanks,
Satya

Attachments:

0001-Prevent-dropping-the-last-label-from-a-property-grap.patchapplication/octet-stream; name=0001-Prevent-dropping-the-last-label-from-a-property-grap.patchDownload+40-1
#2Ashutosh Bapat
ashutosh.bapat@enterprisedb.com
In reply to: SATYANARAYANA NARLAPURAM (#1)
Re: [Bug][patch]: After dropping the last label from a property graph element, invoking pg_get_propgraphdef() triggers an assertion failure

On Mon, Apr 20, 2026 at 11:42 PM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

Hi hackers,

ALTER PROPERTY GRAPH ... ALTER ... DROP LABEL currently allows removing
the last label from an element, leaving it with zero labels.

On assert-enabled builds, pg_get_propgraphdef() hits
TRAP: failed Assert("count > 0"), File: "ruleutils.c", Line: 1837, PID: 1821840

Repro:

CREATE TABLE t (x int PRIMARY KEY, y int, z int);
CREATE PROPERTY GRAPH g VERTEX TABLES (t KEY (x) LABEL l1 LABEL l2);
ALTER PROPERTY GRAPH g ALTER VERTEX TABLE t DROP LABEL l2;
ALTER PROPERTY GRAPH g ALTER VERTEX TABLE t DROP LABEL l1;
SELECT pg_get_propgraphdef('g'::regclass);

We can fix it two ways, (1) Prevent dropping the last label; (2) handle zero labels.
I feel it is easier to prevent dropping the last label than handling zero labels. Thoughts?

SQL/PGQ standard section 11.25 syntax rule 6 says
"Element table descriptor shall include two or more labels, one of
which has an <identifier> that is equivalent to the <label name>
simply contained in the <drop element table label clause>."

IIUC this simply means that the last label can not be dropped. That
agrees with your chosen option.

In the patch,
+ while (HeapTupleIsValid(systable_getnext(elscan)))
+ nlabels++;

It's better to break from the while loop after incrementing nlabels
and avoid scanning the entire table in the worst case. All we want to
check is whether another label exists and not all the labels.

The attached patch adds a check in AlterPropGraph() before
performDeletion(). It scans pg_propgraph_element_label to count labels
for the element, and raises an error if only one remains. A regression test is included
that drops labels down to the last one, verifies the error, then re-adds them back.

I would add a test to make sure ADD LABEL ... DROP LABEL .. is allowed.

While investigating this, I also looked at the DROP PROPERTIES
specification. It's allowed to drop even the last property.

--
Best Wishes,
Ashutosh Bapat

#3SATYANARAYANA NARLAPURAM
satyanarlapuram@gmail.com
In reply to: Ashutosh Bapat (#2)
Re: [Bug][patch]: After dropping the last label from a property graph element, invoking pg_get_propgraphdef() triggers an assertion failure

HI Ashutosh,

On Mon, Apr 20, 2026 at 10:34 PM Ashutosh Bapat <
ashutosh.bapat.oss@gmail.com> wrote:

On Mon, Apr 20, 2026 at 11:42 PM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

Hi hackers,

ALTER PROPERTY GRAPH ... ALTER ... DROP LABEL currently allows removing
the last label from an element, leaving it with zero labels.

On assert-enabled builds, pg_get_propgraphdef() hits
TRAP: failed Assert("count > 0"), File: "ruleutils.c", Line: 1837, PID:

1821840

Repro:

CREATE TABLE t (x int PRIMARY KEY, y int, z int);
CREATE PROPERTY GRAPH g VERTEX TABLES (t KEY (x) LABEL l1 LABEL l2);
ALTER PROPERTY GRAPH g ALTER VERTEX TABLE t DROP LABEL l2;
ALTER PROPERTY GRAPH g ALTER VERTEX TABLE t DROP LABEL l1;
SELECT pg_get_propgraphdef('g'::regclass);

We can fix it two ways, (1) Prevent dropping the last label; (2) handle

zero labels.

I feel it is easier to prevent dropping the last label than handling

zero labels. Thoughts?

SQL/PGQ standard section 11.25 syntax rule 6 says
"Element table descriptor shall include two or more labels, one of
which has an <identifier> that is equivalent to the <label name>
simply contained in the <drop element table label clause>."

IIUC this simply means that the last label can not be dropped. That
agrees with your chosen option.

In the patch,
+ while (HeapTupleIsValid(systable_getnext(elscan)))
+ nlabels++;

It's better to break from the while loop after incrementing nlabels
and avoid scanning the entire table in the worst case. All we want to
check is whether another label exists and not all the labels.

Please find the attached v2 patch.

The attached patch adds a check in AlterPropGraph() before
performDeletion(). It scans pg_propgraph_element_label to count labels
for the element, and raises an error if only one remains. A regression

test is included

that drops labels down to the last one, verifies the error, then re-adds

them back.

I would add a test to make sure ADD LABEL ... DROP LABEL .. is allowed.

+ALTER PROPERTY GRAPH g3 ALTER VERTEX TABLE t3 DROP LABEL t3l1;  -- error:
last label
+ALTER PROPERTY GRAPH g3 ALTER VERTEX TABLE t3 ADD LABEL t3l2 PROPERTIES
ALL COLUMNS;

Are you looking for any additional coverage?

While investigating this, I also looked at the DROP PROPERTIES
specification. It's allowed to drop even the last property.

Thanks for checking this.

Thanks,
Satya

Attachments:

v2-0001-Prevent-dropping-the-last-label-from-a-property-grap.patchapplication/octet-stream; name=v2-0001-Prevent-dropping-the-last-label-from-a-property-grap.patchDownload+44-1
#4Ashutosh Bapat
ashutosh.bapat@enterprisedb.com
In reply to: SATYANARAYANA NARLAPURAM (#3)
Re: [Bug][patch]: After dropping the last label from a property graph element, invoking pg_get_propgraphdef() triggers an assertion failure

On Tue, Apr 21, 2026 at 11:28 AM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

HI Ashutosh,

On Mon, Apr 20, 2026 at 10:34 PM Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> wrote:

On Mon, Apr 20, 2026 at 11:42 PM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

Hi hackers,

ALTER PROPERTY GRAPH ... ALTER ... DROP LABEL currently allows removing
the last label from an element, leaving it with zero labels.

On assert-enabled builds, pg_get_propgraphdef() hits
TRAP: failed Assert("count > 0"), File: "ruleutils.c", Line: 1837, PID: 1821840

Repro:

CREATE TABLE t (x int PRIMARY KEY, y int, z int);
CREATE PROPERTY GRAPH g VERTEX TABLES (t KEY (x) LABEL l1 LABEL l2);
ALTER PROPERTY GRAPH g ALTER VERTEX TABLE t DROP LABEL l2;
ALTER PROPERTY GRAPH g ALTER VERTEX TABLE t DROP LABEL l1;
SELECT pg_get_propgraphdef('g'::regclass);

We can fix it two ways, (1) Prevent dropping the last label; (2) handle zero labels.
I feel it is easier to prevent dropping the last label than handling zero labels. Thoughts?

SQL/PGQ standard section 11.25 syntax rule 6 says
"Element table descriptor shall include two or more labels, one of
which has an <identifier> that is equivalent to the <label name>
simply contained in the <drop element table label clause>."

IIUC this simply means that the last label can not be dropped. That
agrees with your chosen option.

In the patch,
+ while (HeapTupleIsValid(systable_getnext(elscan)))
+ nlabels++;

It's better to break from the while loop after incrementing nlabels
and avoid scanning the entire table in the worst case. All we want to
check is whether another label exists and not all the labels.

Please find the attached v2 patch.

The attached patch adds a check in AlterPropGraph() before
performDeletion(). It scans pg_propgraph_element_label to count labels
for the element, and raises an error if only one remains. A regression test is included
that drops labels down to the last one, verifies the error, then re-adds them back.

I would add a test to make sure ADD LABEL ... DROP LABEL .. is allowed.

+ALTER PROPERTY GRAPH g3 ALTER VERTEX TABLE t3 DROP LABEL t3l1;  -- error: last label
+ALTER PROPERTY GRAPH g3 ALTER VERTEX TABLE t3 ADD LABEL t3l2 PROPERTIES ALL COLUMNS;

Are you looking for any additional coverage?

I thought specifying ADD LABEL and DROP LABEL is supported in the same
DDL. But that's not the case. Sorry.

Will review the patch soon.

Additionally, the patch should update the DDL document to mention that
the DDL does not allow dropping the last label on the given graph
element table.

--
Best Wishes,
Ashutosh Bapat

#5SATYANARAYANA NARLAPURAM
satyanarlapuram@gmail.com
In reply to: Ashutosh Bapat (#4)
Re: [Bug][patch]: After dropping the last label from a property graph element, invoking pg_get_propgraphdef() triggers an assertion failure

Hi,

On Mon, Apr 20, 2026 at 11:58 PM Ashutosh Bapat <
ashutosh.bapat.oss@gmail.com> wrote:

On Tue, Apr 21, 2026 at 11:28 AM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

HI Ashutosh,

On Mon, Apr 20, 2026 at 10:34 PM Ashutosh Bapat <

ashutosh.bapat.oss@gmail.com> wrote:

On Mon, Apr 20, 2026 at 11:42 PM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

Hi hackers,

ALTER PROPERTY GRAPH ... ALTER ... DROP LABEL currently allows

removing

the last label from an element, leaving it with zero labels.

On assert-enabled builds, pg_get_propgraphdef() hits
TRAP: failed Assert("count > 0"), File: "ruleutils.c", Line: 1837,

PID: 1821840

Repro:

CREATE TABLE t (x int PRIMARY KEY, y int, z int);
CREATE PROPERTY GRAPH g VERTEX TABLES (t KEY (x) LABEL l1 LABEL l2);
ALTER PROPERTY GRAPH g ALTER VERTEX TABLE t DROP LABEL l2;
ALTER PROPERTY GRAPH g ALTER VERTEX TABLE t DROP LABEL l1;
SELECT pg_get_propgraphdef('g'::regclass);

We can fix it two ways, (1) Prevent dropping the last label; (2)

handle zero labels.

I feel it is easier to prevent dropping the last label than handling

zero labels. Thoughts?

SQL/PGQ standard section 11.25 syntax rule 6 says
"Element table descriptor shall include two or more labels, one of
which has an <identifier> that is equivalent to the <label name>
simply contained in the <drop element table label clause>."

IIUC this simply means that the last label can not be dropped. That
agrees with your chosen option.

In the patch,
+ while (HeapTupleIsValid(systable_getnext(elscan)))
+ nlabels++;

It's better to break from the while loop after incrementing nlabels
and avoid scanning the entire table in the worst case. All we want to
check is whether another label exists and not all the labels.

Please find the attached v2 patch.

The attached patch adds a check in AlterPropGraph() before
performDeletion(). It scans pg_propgraph_element_label to count labels
for the element, and raises an error if only one remains. A

regression test is included

that drops labels down to the last one, verifies the error, then

re-adds them back.

I would add a test to make sure ADD LABEL ... DROP LABEL .. is allowed.

+ALTER PROPERTY GRAPH g3 ALTER VERTEX TABLE t3 DROP LABEL t3l1; --

error: last label

+ALTER PROPERTY GRAPH g3 ALTER VERTEX TABLE t3 ADD LABEL t3l2 PROPERTIES

ALL COLUMNS;

Are you looking for any additional coverage?

I thought specifying ADD LABEL and DROP LABEL is supported in the same
DDL. But that's not the case. Sorry.

Will review the patch soon.

Additionally, the patch should update the DDL document to mention that
the DDL does not allow dropping the last label on the given graph
element table.

Addressed this in v3 patch.

Thanks,
Satya

Attachments:

v3-0001-Prevent-dropping-the-last-label-from-a-property-grap.patchapplication/octet-stream; name=v3-0001-Prevent-dropping-the-last-label-from-a-property-grap.patchDownload+46-1
#6Ashutosh Bapat
ashutosh.bapat@enterprisedb.com
In reply to: SATYANARAYANA NARLAPURAM (#5)
Re: [Bug][patch]: After dropping the last label from a property graph element, invoking pg_get_propgraphdef() triggers an assertion failure

On Tue, Apr 21, 2026 at 1:29 PM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

Hi,

On Mon, Apr 20, 2026 at 11:58 PM Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> wrote:

On Tue, Apr 21, 2026 at 11:28 AM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

HI Ashutosh,

On Mon, Apr 20, 2026 at 10:34 PM Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> wrote:

On Mon, Apr 20, 2026 at 11:42 PM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

Hi hackers,

ALTER PROPERTY GRAPH ... ALTER ... DROP LABEL currently allows removing
the last label from an element, leaving it with zero labels.

On assert-enabled builds, pg_get_propgraphdef() hits
TRAP: failed Assert("count > 0"), File: "ruleutils.c", Line: 1837, PID: 1821840

Repro:

CREATE TABLE t (x int PRIMARY KEY, y int, z int);
CREATE PROPERTY GRAPH g VERTEX TABLES (t KEY (x) LABEL l1 LABEL l2);
ALTER PROPERTY GRAPH g ALTER VERTEX TABLE t DROP LABEL l2;
ALTER PROPERTY GRAPH g ALTER VERTEX TABLE t DROP LABEL l1;
SELECT pg_get_propgraphdef('g'::regclass);

We can fix it two ways, (1) Prevent dropping the last label; (2) handle zero labels.
I feel it is easier to prevent dropping the last label than handling zero labels. Thoughts?

SQL/PGQ standard section 11.25 syntax rule 6 says
"Element table descriptor shall include two or more labels, one of
which has an <identifier> that is equivalent to the <label name>
simply contained in the <drop element table label clause>."

IIUC this simply means that the last label can not be dropped. That
agrees with your chosen option.

In the patch,
+ while (HeapTupleIsValid(systable_getnext(elscan)))
+ nlabels++;

It's better to break from the while loop after incrementing nlabels
and avoid scanning the entire table in the worst case. All we want to
check is whether another label exists and not all the labels.

Please find the attached v2 patch.

The attached patch adds a check in AlterPropGraph() before
performDeletion(). It scans pg_propgraph_element_label to count labels
for the element, and raises an error if only one remains. A regression test is included
that drops labels down to the last one, verifies the error, then re-adds them back.

I would add a test to make sure ADD LABEL ... DROP LABEL .. is allowed.

+ALTER PROPERTY GRAPH g3 ALTER VERTEX TABLE t3 DROP LABEL t3l1;  -- error: last label
+ALTER PROPERTY GRAPH g3 ALTER VERTEX TABLE t3 ADD LABEL t3l2 PROPERTIES ALL COLUMNS;

Are you looking for any additional coverage?

I thought specifying ADD LABEL and DROP LABEL is supported in the same
DDL. But that's not the case. Sorry.

Will review the patch soon.

Additionally, the patch should update the DDL document to mention that
the DDL does not allow dropping the last label on the given graph
element table.

Addressed this in v3 patch.

Thanks.

I questioned whether it makes sense to move the code to check the
existence of at least one other label into its own function so the
code is more readable. AlterPropGraph is already about 500 lines and
this code increases it further. There are two possibilities:
a. The function returns true if the given element has more than two
labels associated with it. The choice of 2 makes sense only in the
drop label context but not otherwise.
b. The function returns true if it finds one label other than the one
with the given label oid. That looks a bit more elegant. But still
seems too tied to drop label context.

So I decided against it. But instead removed the extra indentation.
The earliest variable declaration block is not farther from that code.

I changed the errcode to ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE
since we expect the element to be in a specific state to drop the
label. ERRCODE_INVALID_OBJECT_DEFINITION would mean that the
definition of the element or the label is itself invalid. That's not
the case.

Here's a patch with some minor edits.

--
Best Wishes,
Ashutosh Bapat

Attachments:

v20260421-0001-Prevent-dropping-the-last-label-from-a-pro.patchtext/x-patch; charset=US-ASCII; name=v20260421-0001-Prevent-dropping-the-last-label-from-a-pro.patchDownload+45-2
#7Ashutosh Bapat
ashutosh.bapat@enterprisedb.com
In reply to: Ashutosh Bapat (#6)
Re: [Bug][patch]: After dropping the last label from a property graph element, invoking pg_get_propgraphdef() triggers an assertion failure

On Tue, Apr 21, 2026 at 5:49 PM Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:

On Tue, Apr 21, 2026 at 1:29 PM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

Hi,

On Mon, Apr 20, 2026 at 11:58 PM Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> wrote:

On Tue, Apr 21, 2026 at 11:28 AM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

HI Ashutosh,

On Mon, Apr 20, 2026 at 10:34 PM Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> wrote:

On Mon, Apr 20, 2026 at 11:42 PM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

Hi hackers,

ALTER PROPERTY GRAPH ... ALTER ... DROP LABEL currently allows removing
the last label from an element, leaving it with zero labels.

On assert-enabled builds, pg_get_propgraphdef() hits
TRAP: failed Assert("count > 0"), File: "ruleutils.c", Line: 1837, PID: 1821840

Repro:

CREATE TABLE t (x int PRIMARY KEY, y int, z int);
CREATE PROPERTY GRAPH g VERTEX TABLES (t KEY (x) LABEL l1 LABEL l2);
ALTER PROPERTY GRAPH g ALTER VERTEX TABLE t DROP LABEL l2;
ALTER PROPERTY GRAPH g ALTER VERTEX TABLE t DROP LABEL l1;
SELECT pg_get_propgraphdef('g'::regclass);

We can fix it two ways, (1) Prevent dropping the last label; (2) handle zero labels.
I feel it is easier to prevent dropping the last label than handling zero labels. Thoughts?

SQL/PGQ standard section 11.25 syntax rule 6 says
"Element table descriptor shall include two or more labels, one of
which has an <identifier> that is equivalent to the <label name>
simply contained in the <drop element table label clause>."

IIUC this simply means that the last label can not be dropped. That
agrees with your chosen option.

In the patch,
+ while (HeapTupleIsValid(systable_getnext(elscan)))
+ nlabels++;

It's better to break from the while loop after incrementing nlabels
and avoid scanning the entire table in the worst case. All we want to
check is whether another label exists and not all the labels.

Please find the attached v2 patch.

The attached patch adds a check in AlterPropGraph() before
performDeletion(). It scans pg_propgraph_element_label to count labels
for the element, and raises an error if only one remains. A regression test is included
that drops labels down to the last one, verifies the error, then re-adds them back.

I would add a test to make sure ADD LABEL ... DROP LABEL .. is allowed.

+ALTER PROPERTY GRAPH g3 ALTER VERTEX TABLE t3 DROP LABEL t3l1;  -- error: last label
+ALTER PROPERTY GRAPH g3 ALTER VERTEX TABLE t3 ADD LABEL t3l2 PROPERTIES ALL COLUMNS;

Are you looking for any additional coverage?

I thought specifying ADD LABEL and DROP LABEL is supported in the same
DDL. But that's not the case. Sorry.

Will review the patch soon.

Additionally, the patch should update the DDL document to mention that
the DDL does not allow dropping the last label on the given graph
element table.

Addressed this in v3 patch.

Thanks.

I questioned whether it makes sense to move the code to check the
existence of at least one other label into its own function so the
code is more readable. AlterPropGraph is already about 500 lines and
this code increases it further. There are two possibilities:
a. The function returns true if the given element has more than two
labels associated with it. The choice of 2 makes sense only in the
drop label context but not otherwise.
b. The function returns true if it finds one label other than the one
with the given label oid. That looks a bit more elegant. But still
seems too tied to drop label context.

So I decided against it. But instead removed the extra indentation.
The earliest variable declaration block is not farther from that code.

I changed the errcode to ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE
since we expect the element to be in a specific state to drop the
label. ERRCODE_INVALID_OBJECT_DEFINITION would mean that the
definition of the element or the label is itself invalid. That's not
the case.

Here's a patch with some minor edits.

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.

This patch causes a conflict with the patch posted at [1]/messages/by-id/E3D6552E-DB26-47BC-9C7C-0F9CB936BF5B@gmail.com. Hence
posting both the patches together here, so that they can be committed
without conflict.

[1]: /messages/by-id/E3D6552E-DB26-47BC-9C7C-0F9CB936BF5B@gmail.com

--
Best Wishes,
Ashutosh Bapat

Attachments:

v20260428-0001-Prevent-dropping-the-last-label-from-a-pro.patchtext/x-patch; charset=US-ASCII; name=v20260428-0001-Prevent-dropping-the-last-label-from-a-pro.patchDownload+62-7
v20260428-0002-Dropping-a-property-not-associated-with-th.patchtext/x-patch; charset=US-ASCII; name=v20260428-0002-Dropping-a-property-not-associated-with-th.patchDownload+22-29
#8Chao Li
li.evan.chao@gmail.com
In reply to: Ashutosh Bapat (#7)
Re: [Bug][patch]: After dropping the last label from a property graph element, invoking pg_get_propgraphdef() triggers an assertion failure

On Apr 28, 2026, at 23:02, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> wrote:

On Tue, Apr 21, 2026 at 5:49 PM Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:

On Tue, Apr 21, 2026 at 1:29 PM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

Hi,

On Mon, Apr 20, 2026 at 11:58 PM Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> wrote:

On Tue, Apr 21, 2026 at 11:28 AM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

HI Ashutosh,

On Mon, Apr 20, 2026 at 10:34 PM Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> wrote:

On Mon, Apr 20, 2026 at 11:42 PM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

Hi hackers,

ALTER PROPERTY GRAPH ... ALTER ... DROP LABEL currently allows removing
the last label from an element, leaving it with zero labels.

On assert-enabled builds, pg_get_propgraphdef() hits
TRAP: failed Assert("count > 0"), File: "ruleutils.c", Line: 1837, PID: 1821840

Repro:

CREATE TABLE t (x int PRIMARY KEY, y int, z int);
CREATE PROPERTY GRAPH g VERTEX TABLES (t KEY (x) LABEL l1 LABEL l2);
ALTER PROPERTY GRAPH g ALTER VERTEX TABLE t DROP LABEL l2;
ALTER PROPERTY GRAPH g ALTER VERTEX TABLE t DROP LABEL l1;
SELECT pg_get_propgraphdef('g'::regclass);

We can fix it two ways, (1) Prevent dropping the last label; (2) handle zero labels.
I feel it is easier to prevent dropping the last label than handling zero labels. Thoughts?

SQL/PGQ standard section 11.25 syntax rule 6 says
"Element table descriptor shall include two or more labels, one of
which has an <identifier> that is equivalent to the <label name>
simply contained in the <drop element table label clause>."

IIUC this simply means that the last label can not be dropped. That
agrees with your chosen option.

In the patch,
+ while (HeapTupleIsValid(systable_getnext(elscan)))
+ nlabels++;

It's better to break from the while loop after incrementing nlabels
and avoid scanning the entire table in the worst case. All we want to
check is whether another label exists and not all the labels.

Please find the attached v2 patch.

The attached patch adds a check in AlterPropGraph() before
performDeletion(). It scans pg_propgraph_element_label to count labels
for the element, and raises an error if only one remains. A regression test is included
that drops labels down to the last one, verifies the error, then re-adds them back.

I would add a test to make sure ADD LABEL ... DROP LABEL .. is allowed.

+ALTER PROPERTY GRAPH g3 ALTER VERTEX TABLE t3 DROP LABEL t3l1;  -- error: last label
+ALTER PROPERTY GRAPH g3 ALTER VERTEX TABLE t3 ADD LABEL t3l2 PROPERTIES ALL COLUMNS;

Are you looking for any additional coverage?

I thought specifying ADD LABEL and DROP LABEL is supported in the same
DDL. But that's not the case. Sorry.

Will review the patch soon.

Additionally, the patch should update the DDL document to mention that
the DDL does not allow dropping the last label on the given graph
element table.

Addressed this in v3 patch.

Thanks.

I questioned whether it makes sense to move the code to check the
existence of at least one other label into its own function so the
code is more readable. AlterPropGraph is already about 500 lines and
this code increases it further. There are two possibilities:
a. The function returns true if the given element has more than two
labels associated with it. The choice of 2 makes sense only in the
drop label context but not otherwise.
b. The function returns true if it finds one label other than the one
with the given label oid. That looks a bit more elegant. But still
seems too tied to drop label context.

So I decided against it. But instead removed the extra indentation.
The earliest variable declaration block is not farther from that code.

I changed the errcode to ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE
since we expect the element to be in a specific state to drop the
label. ERRCODE_INVALID_OBJECT_DEFINITION would mean that the
definition of the element or the label is itself invalid. That's not
the case.

Here's a patch with some minor edits.

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.

This patch causes a conflict with the patch posted at [1]. Hence
posting both the patches together here, so that they can be committed
without conflict.

[1] /messages/by-id/E3D6552E-DB26-47BC-9C7C-0F9CB936BF5B@gmail.com

--
Best Wishes,
Ashutosh Bapat
<v20260428-0001-Prevent-dropping-the-last-label-from-a-pro.patch><v20260428-0002-Dropping-a-property-not-associated-with-th.patch>

As Alvaro commented at [1]/messages/by-id/02fe13db-4fba-4e9d-9b4c-e6271a133502@app.fastmail.com, I added 0003 to use OidIsValid macro. 0001 and 0002 are unchanged.

[1]: /messages/by-id/02fe13db-4fba-4e9d-9b4c-e6271a133502@app.fastmail.com

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

Attachments:

v20260429-0001-Prevent-dropping-the-last-label-from-a-pro.patchapplication/octet-stream; name=v20260429-0001-Prevent-dropping-the-last-label-from-a-pro.patch; x-unix-mode=0644Download+62-7
v20260429-0002-Dropping-a-property-not-associated-with-th.patchapplication/octet-stream; name=v20260429-0002-Dropping-a-property-not-associated-with-th.patch; x-unix-mode=0644Download+22-29
v20260429-0003-Use-OidIsValid-macro-in-propgraphcmds.c.patchapplication/octet-stream; name=v20260429-0003-Use-OidIsValid-macro-in-propgraphcmds.c.patch; x-unix-mode=0644Download+22-23