MIME-Version: 1.0
References: 
 <CAHg+QDeP=mTHTV48R23zKMy1SBmCKZ_L7-z5zKnYyw+K0x-gCg@mail.gmail.com>
 <CAExHW5tsOK7ja2dMykD_qQSLwsuZxCTHx_jFyx-7-O+9WRBDXQ@mail.gmail.com>
In-Reply-To: 
 <CAExHW5tsOK7ja2dMykD_qQSLwsuZxCTHx_jFyx-7-O+9WRBDXQ@mail.gmail.com>
From: SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com>
Date: Mon, 20 Apr 2026 22:58:40 -0700
Message-ID: 
 <CAHg+QDcXxmeiuySsyR5K=8Wk9Pq2sZ_4HNb6EMonN=yjLz_fyw@mail.gmail.com>
Subject: Re: [Bug][patch]: After dropping the last label from a property graph
 element, invoking pg_get_propgraphdef() triggers an assertion failure
To: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: multipart/mixed; boundary="0000000000001d8da6064ff21cbe"
Archived-At: 
 <https://www.postgresql.org/message-id/CAHg%2BQDcXxmeiuySsyR5K%3D8Wk9Pq2sZ_4HNb6EMonN%3DyjLz_fyw%40mail.gmail.com>
Precedence: bulk

--0000000000001d8da6064ff21cbe
Content-Type: multipart/alternative; boundary="0000000000001d8da5064ff21cbc"

--0000000000001d8da5064ff21cbc
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

HI Ashutosh,

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

> On Mon, Apr 20, 2026 at 11:42=E2=80=AFPM 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-add=
s
> 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

--0000000000001d8da5064ff21cbc
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr">HI Ashutosh,</div><br><div class=3D"gmail=
_quote gmail_quote_container"><div dir=3D"ltr" class=3D"gmail_attr">On Mon,=
 Apr 20, 2026 at 10:34=E2=80=AFPM Ashutosh Bapat &lt;<a href=3D"mailto:ashu=
tosh.bapat.oss@gmail.com">ashutosh.bapat.oss@gmail.com</a>&gt; wrote:<br></=
div><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;bor=
der-left:1px solid rgb(204,204,204);padding-left:1ex">On Mon, Apr 20, 2026 =
at 11:42=E2=80=AFPM SATYANARAYANA NARLAPURAM<br>
&lt;<a href=3D"mailto:satyanarlapuram@gmail.com" target=3D"_blank">satyanar=
lapuram@gmail.com</a>&gt; wrote:<br>
&gt;<br>
&gt; Hi hackers,<br>
&gt;<br>
&gt; ALTER PROPERTY GRAPH ... ALTER ... DROP LABEL currently allows removin=
g<br>
&gt; the last label from an element, leaving it with zero labels.<br>
&gt;<br>
&gt; On assert-enabled builds, pg_get_propgraphdef() hits<br>
&gt; TRAP: failed Assert(&quot;count &gt; 0&quot;), File: &quot;ruleutils.c=
&quot;, Line: 1837, PID: 1821840<br>
&gt;<br>
&gt; Repro:<br>
&gt;<br>
&gt; CREATE TABLE t (x int PRIMARY KEY, y int, z int);<br>
&gt; CREATE PROPERTY GRAPH g VERTEX TABLES (t KEY (x) LABEL l1 LABEL l2);<b=
r>
&gt; ALTER PROPERTY GRAPH g ALTER VERTEX TABLE t DROP LABEL l2;<br>
&gt; ALTER PROPERTY GRAPH g ALTER VERTEX TABLE t DROP LABEL l1;<br>
&gt; SELECT pg_get_propgraphdef(&#39;g&#39;::regclass);<br>
&gt;<br>
&gt; We can fix it two ways, (1) Prevent dropping the last label; (2) handl=
e zero labels.<br>
&gt; I feel it is easier to prevent dropping the last label than handling z=
ero labels. Thoughts?<br>
&gt;<br>
<br>
SQL/PGQ standard section 11.25 syntax rule 6 says<br>
&quot;Element table descriptor shall include two or more labels, one of<br>
which has an &lt;identifier&gt; that is equivalent to the &lt;label name&gt=
;<br>
simply contained in the &lt;drop element table label clause&gt;.&quot;<br>
<br>
IIUC this simply means that the last label can not be dropped. That<br>
agrees with your chosen option.<br>
<br>
In the patch,<br>
+ while (HeapTupleIsValid(systable_getnext(elscan)))<br>
+ nlabels++;<br>
<br>
It&#39;s better to break from the while loop after incrementing nlabels<br>
and avoid scanning the entire table in the worst case. All we want to<br>
check is whether another label exists and not all the labels.<br></blockquo=
te><div><br></div><div>Please find the attached v2 patch.</div><div>=C2=A0<=
/div><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;bo=
rder-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
&gt; The attached patch adds a check in AlterPropGraph() before<br>
&gt; performDeletion(). It scans pg_propgraph_element_label to count labels=
<br>
&gt; for the element, and raises an error if only one remains. A regression=
 test is included<br>
&gt; that drops labels down to the last one, verifies the error, then re-ad=
ds them back.<br>
<br>
I would add a test to make sure ADD LABEL ... DROP LABEL .. is allowed.<br>=
</blockquote><div><br></div><div>+ALTER PROPERTY GRAPH g3 ALTER VERTEX TABL=
E t3 DROP LABEL t3l1; =C2=A0-- error: last label<br>+ALTER PROPERTY GRAPH g=
3 ALTER VERTEX TABLE t3 ADD LABEL t3l2 PROPERTIES ALL COLUMNS;</div><div><b=
r></div><div>Are you looking for any additional coverage?</div><div><br></d=
iv><div>=C2=A0</div><blockquote class=3D"gmail_quote" style=3D"margin:0px 0=
px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
While investigating this, I also looked at the DROP PROPERTIES<br>
specification. It&#39;s allowed to drop even the last property.<br></blockq=
uote><div><br></div><div>Thanks for checking this.</div><div><br></div><div=
>Thanks,</div><div>Satya=C2=A0</div></div></div>

--0000000000001d8da5064ff21cbc--

--0000000000001d8da6064ff21cbe
Content-Type: application/octet-stream;
	name="v2-0001-Prevent-dropping-the-last-label-from-a-property-grap.patch"
Content-Disposition: attachment;
	filename="v2-0001-Prevent-dropping-the-last-label-from-a-property-grap.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_mo87o8oz0>
X-Attachment-Id: f_mo87o8oz0

RnJvbSAxNmYwMmFlNWQ4ZjY4NTRmZTRmOWRkMzE4MTUxYjRlMmU0ZTViODYxIE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBzYXR5YW5hcmF5YW5hIG5hcmxhcHVyYW0gPHNhdHlhbmFybGFw
dXJhbUBnbWFpbC5jb20+CkRhdGU6IFR1ZSwgMjEgQXByIDIwMjYgMDU6NDk6NTEgKzAwMDAKU3Vi
amVjdDogW1BBVENIXSBQcmV2ZW50IGRyb3BwaW5nIHRoZSBsYXN0IGxhYmVsIGZyb20gYSBwcm9w
ZXJ0eSBncmFwaCBlbGVtZW50CgpBTFRFUiBQUk9QRVJUWSBHUkFQSCAuLi4gRFJPUCBMQUJFTCBh
bGxvd2VkIHJlbW92aW5nIHRoZSBsYXN0IGxhYmVsCmZyb20gYW4gZWxlbWVudCwgbGVhdmluZyBp
dCB3aXRoIHplcm8gbGFiZWxzLiBUaGlzIGNhdXNlcwpwZ19nZXRfcHJvcGdyYXBoZGVmKCkgdG8g
Y3Jhc2guCgpBZGQgYSBjaGVjayBpbiBBbHRlclByb3BHcmFwaCgpIHRoYXQgY291bnRzIHRoZSBl
bGVtZW50J3MgbGFiZWxzCmJlZm9yZSBkZWxldGlvbiBhbmQgZXJyb3JzIG91dCBpZiBvbmx5IG9u
ZSByZW1haW5zLiAgVGhlIGxhYmVsIGNvdW50Cmxvb3AgYnJlYWtzIGVhcmx5IG9uY2UgaXQgZmlu
ZHMgbW9yZSB0aGFuIG9uZSBsYWJlbCwgYXZvaWRpbmcgYSBmdWxsCnNjYW4gb2YgdGhlIGVsZW1l
bnRfbGFiZWwgY2F0YWxvZy4KCi0tLQogc3JjL2JhY2tlbmQvY29tbWFuZHMvcHJvcGdyYXBoY21k
cy5jICAgICAgICAgIHwgMzQgKysrKysrKysrKysrKysrKysrKwogLi4uL2V4cGVjdGVkL2NyZWF0
ZV9wcm9wZXJ0eV9ncmFwaC5vdXQgICAgICAgIHwgIDYgKysrKwogLi4uL3JlZ3Jlc3Mvc3FsL2Ny
ZWF0ZV9wcm9wZXJ0eV9ncmFwaC5zcWwgICAgIHwgIDQgKysrCiAzIGZpbGVzIGNoYW5nZWQsIDQ0
IGluc2VydGlvbnMoKykKCmRpZmYgLS1naXQgYS9zcmMvYmFja2VuZC9jb21tYW5kcy9wcm9wZ3Jh
cGhjbWRzLmMgYi9zcmMvYmFja2VuZC9jb21tYW5kcy9wcm9wZ3JhcGhjbWRzLmMKaW5kZXggNDVk
MmZmMWIuLmIxYzVlOGFlIDEwMDY0NAotLS0gYS9zcmMvYmFja2VuZC9jb21tYW5kcy9wcm9wZ3Jh
cGhjbWRzLmMKKysrIGIvc3JjL2JhY2tlbmQvY29tbWFuZHMvcHJvcGdyYXBoY21kcy5jCkBAIC0x
NTIyLDYgKzE1MjIsNDAgQEAgQWx0ZXJQcm9wR3JhcGgoUGFyc2VTdGF0ZSAqcHN0YXRlLCBjb25z
dCBBbHRlclByb3BHcmFwaFN0bXQgKnN0bXQpCiAJCQkJCQkgICBnZXRfcmVsX25hbWUocGdyZWxp
ZCksIHN0bXQtPmVsZW1lbnRfYWxpYXMsIHN0bXQtPmRyb3BfbGFiZWwpLAogCQkJCQlwYXJzZXJf
ZXJycG9zaXRpb24ocHN0YXRlLCAtMSkpOwogCisJCS8qCisJCSAqIFByZXZlbnQgZHJvcHBpbmcg
dGhlIGxhc3QgbGFiZWwgZnJvbSBhbiBlbGVtZW50LiBFdmVyeSBlbGVtZW50CisJCSAqIG11c3Qg
aGF2ZSBhdCBsZWFzdCBvbmUgbGFiZWwuCisJCSAqLworCQl7CisJCQlSZWxhdGlvbgllbHJlbDsK
KwkJCVN5c1NjYW5EZXNjIGVsc2NhbjsKKwkJCVNjYW5LZXlEYXRhIGVsa2V5WzFdOworCQkJaW50
CQkJbmxhYmVscyA9IDA7CisKKwkJCWVscmVsID0gdGFibGVfb3BlbihQcm9wZ3JhcGhFbGVtZW50
TGFiZWxSZWxhdGlvbklkLCBBY2Nlc3NTaGFyZUxvY2spOworCQkJU2NhbktleUluaXQoJmVsa2V5
WzBdLAorCQkJCQkJQW51bV9wZ19wcm9wZ3JhcGhfZWxlbWVudF9sYWJlbF9wZ2VsZWxpZCwKKwkJ
CQkJCUJURXF1YWxTdHJhdGVneU51bWJlciwgRl9PSURFUSwKKwkJCQkJCU9iamVjdElkR2V0RGF0
dW0ocGVvaWQpKTsKKwkJCWVsc2NhbiA9IHN5c3RhYmxlX2JlZ2luc2NhbihlbHJlbCwgUHJvcGdy
YXBoRWxlbWVudExhYmVsRWxlbWVudExhYmVsSW5kZXhJZCwKKwkJCQkJCQkJCQl0cnVlLCBOVUxM
LCAxLCBlbGtleSk7CisJCQl3aGlsZSAoSGVhcFR1cGxlSXNWYWxpZChzeXN0YWJsZV9nZXRuZXh0
KGVsc2NhbikpKQorCQkJeworCQkJCW5sYWJlbHMrKzsKKwkJCQlpZiAobmxhYmVscyA+IDEpCisJ
CQkJCWJyZWFrOworCQkJfQorCQkJc3lzdGFibGVfZW5kc2NhbihlbHNjYW4pOworCQkJdGFibGVf
Y2xvc2UoZWxyZWwsIEFjY2Vzc1NoYXJlTG9jayk7CisKKwkJCWlmIChubGFiZWxzIDw9IDEpCisJ
CQkJZXJlcG9ydChFUlJPUiwKKwkJCQkJCShlcnJjb2RlKEVSUkNPREVfSU5WQUxJRF9PQkpFQ1Rf
REVGSU5JVElPTiksCisJCQkJCQkgZXJybXNnKCJjYW5ub3QgZHJvcCB0aGUgbGFzdCBsYWJlbCBm
cm9tIGVsZW1lbnQgXCIlc1wiIiwKKwkJCQkJCQkJc3RtdC0+ZWxlbWVudF9hbGlhcyksCisJCQkJ
CQkgZXJyaGludCgiRXZlcnkgZWxlbWVudCBtdXN0IGhhdmUgYXQgbGVhc3Qgb25lIGxhYmVsLiIp
KSk7CisJCX0KKwogCQlPYmplY3RBZGRyZXNzU2V0KG9iaiwgUHJvcGdyYXBoRWxlbWVudExhYmVs
UmVsYXRpb25JZCwgZWxsYWJlbG9pZCk7CiAJCXBlcmZvcm1EZWxldGlvbigmb2JqLCBzdG10LT5k
cm9wX2JlaGF2aW9yLCAwKTsKIApkaWZmIC0tZ2l0IGEvc3JjL3Rlc3QvcmVncmVzcy9leHBlY3Rl
ZC9jcmVhdGVfcHJvcGVydHlfZ3JhcGgub3V0IGIvc3JjL3Rlc3QvcmVncmVzcy9leHBlY3RlZC9j
cmVhdGVfcHJvcGVydHlfZ3JhcGgub3V0CmluZGV4IGJjOWE1OTZlLi43NDBmODg2YyAxMDA2NDQK
LS0tIGEvc3JjL3Rlc3QvcmVncmVzcy9leHBlY3RlZC9jcmVhdGVfcHJvcGVydHlfZ3JhcGgub3V0
CisrKyBiL3NyYy90ZXN0L3JlZ3Jlc3MvZXhwZWN0ZWQvY3JlYXRlX3Byb3BlcnR5X2dyYXBoLm91
dApAQCAtNTcsNiArNTcsMTIgQEAgQUxURVIgUFJPUEVSVFkgR1JBUEggZzMKIEFMVEVSIFBST1BF
UlRZIEdSQVBIIGczIEFMVEVSIFZFUlRFWCBUQUJMRSB0MyBEUk9QIExBQkVMIHQzbDN4OyAgLS0g
ZXJyb3IKIEVSUk9SOiAgcHJvcGVydHkgZ3JhcGggImczIiBlbGVtZW50ICJ0MyIgaGFzIG5vIGxh
YmVsICJ0M2wzeCIKIEFMVEVSIFBST1BFUlRZIEdSQVBIIGczIEFMVEVSIFZFUlRFWCBUQUJMRSB0
MyBEUk9QIExBQkVMIHQzbDM7CistLSBUZXN0IHRoYXQgdGhlIGxhc3QgbGFiZWwgb24gYW4gZWxl
bWVudCBjYW5ub3QgYmUgZHJvcHBlZAorQUxURVIgUFJPUEVSVFkgR1JBUEggZzMgQUxURVIgVkVS
VEVYIFRBQkxFIHQzIERST1AgTEFCRUwgdDNsMjsKK0FMVEVSIFBST1BFUlRZIEdSQVBIIGczIEFM
VEVSIFZFUlRFWCBUQUJMRSB0MyBEUk9QIExBQkVMIHQzbDE7ICAtLSBlcnJvcjogbGFzdCBsYWJl
bAorRVJST1I6ICBjYW5ub3QgZHJvcCB0aGUgbGFzdCBsYWJlbCBmcm9tIGVsZW1lbnQgInQzIgor
SElOVDogIEV2ZXJ5IGVsZW1lbnQgbXVzdCBoYXZlIGF0IGxlYXN0IG9uZSBsYWJlbC4KK0FMVEVS
IFBST1BFUlRZIEdSQVBIIGczIEFMVEVSIFZFUlRFWCBUQUJMRSB0MyBBREQgTEFCRUwgdDNsMiBQ
Uk9QRVJUSUVTIEFMTCBDT0xVTU5TOwogQUxURVIgUFJPUEVSVFkgR1JBUEggZzMgRFJPUCBWRVJU
RVggVEFCTEVTICh0Mik7ICAtLSBmYWlsCiBFUlJPUjogIGNhbm5vdCBkcm9wIHZlcnRleCB0MiBv
ZiBwcm9wZXJ0eSBncmFwaCBnMyBiZWNhdXNlIG90aGVyIG9iamVjdHMgZGVwZW5kIG9uIGl0CiBE
RVRBSUw6ICBlZGdlIGUxIG9mIHByb3BlcnR5IGdyYXBoIGczIGRlcGVuZHMgb24gdmVydGV4IHQy
IG9mIHByb3BlcnR5IGdyYXBoIGczCmRpZmYgLS1naXQgYS9zcmMvdGVzdC9yZWdyZXNzL3NxbC9j
cmVhdGVfcHJvcGVydHlfZ3JhcGguc3FsIGIvc3JjL3Rlc3QvcmVncmVzcy9zcWwvY3JlYXRlX3By
b3BlcnR5X2dyYXBoLnNxbAppbmRleCAyNDFmOTNkZi4uNGNmNzcxNTkgMTAwNjQ0Ci0tLSBhL3Ny
Yy90ZXN0L3JlZ3Jlc3Mvc3FsL2NyZWF0ZV9wcm9wZXJ0eV9ncmFwaC5zcWwKKysrIGIvc3JjL3Rl
c3QvcmVncmVzcy9zcWwvY3JlYXRlX3Byb3BlcnR5X2dyYXBoLnNxbApAQCAtNTIsNiArNTIsMTAg
QEAgQUxURVIgUFJPUEVSVFkgR1JBUEggZzMKICAgICAgICAgQUREIExBQkVMIHQzbDMgUFJPUEVS
VElFUyBBTEwgQ09MVU1OUzsKIEFMVEVSIFBST1BFUlRZIEdSQVBIIGczIEFMVEVSIFZFUlRFWCBU
QUJMRSB0MyBEUk9QIExBQkVMIHQzbDN4OyAgLS0gZXJyb3IKIEFMVEVSIFBST1BFUlRZIEdSQVBI
IGczIEFMVEVSIFZFUlRFWCBUQUJMRSB0MyBEUk9QIExBQkVMIHQzbDM7CistLSBUZXN0IHRoYXQg
dGhlIGxhc3QgbGFiZWwgb24gYW4gZWxlbWVudCBjYW5ub3QgYmUgZHJvcHBlZAorQUxURVIgUFJP
UEVSVFkgR1JBUEggZzMgQUxURVIgVkVSVEVYIFRBQkxFIHQzIERST1AgTEFCRUwgdDNsMjsKK0FM
VEVSIFBST1BFUlRZIEdSQVBIIGczIEFMVEVSIFZFUlRFWCBUQUJMRSB0MyBEUk9QIExBQkVMIHQz
bDE7ICAtLSBlcnJvcjogbGFzdCBsYWJlbAorQUxURVIgUFJPUEVSVFkgR1JBUEggZzMgQUxURVIg
VkVSVEVYIFRBQkxFIHQzIEFERCBMQUJFTCB0M2wyIFBST1BFUlRJRVMgQUxMIENPTFVNTlM7CiBB
TFRFUiBQUk9QRVJUWSBHUkFQSCBnMyBEUk9QIFZFUlRFWCBUQUJMRVMgKHQyKTsgIC0tIGZhaWwK
IEFMVEVSIFBST1BFUlRZIEdSQVBIIGczIERST1AgVkVSVEVYIFRBQkxFUyAodDIpIENBU0NBREU7
CiBBTFRFUiBQUk9QRVJUWSBHUkFQSCBnMyBEUk9QIEVER0UgVEFCTEVTIChlMik7Ci0tIAoyLjQz
LjAKCg==
--0000000000001d8da6064ff21cbe--