public inbox for [email protected]  

help / color / mirror / Atom feed

From: Ashutosh Bapat <[email protected]>
To: SATYANARAYANA NARLAPURAM <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: [Bug][patch]: After dropping the last label from a property graph element, invoking pg_get_propgraphdef() triggers an assertion failure
Date: Tue, 21 Apr 2026 12:28:08 +0530
Message-ID: <CAExHW5t2YtTMnK7qbvL++pBet-5B8bzgU+8wMcC5aqZwBH0C9w@mail.gmail.com> (raw)
In-Reply-To: <CAHg+QDcXxmeiuySsyR5K=8Wk9Pq2sZ_4HNb6EMonN=yjLz_fyw@mail.gmail.com>
References: <CAHg+QDeP=mTHTV48R23zKMy1SBmCKZ_L7-z5zKnYyw+K0x-gCg@mail.gmail.com>
	<CAExHW5tsOK7ja2dMykD_qQSLwsuZxCTHx_jFyx-7-O+9WRBDXQ@mail.gmail.com>
	<CAHg+QDcXxmeiuySsyR5K=8Wk9Pq2sZ_4HNb6EMonN=yjLz_fyw@mail.gmail.com>

On Tue, Apr 21, 2026 at 11:28 AM SATYANARAYANA NARLAPURAM
<[email protected]> wrote:
>
> HI Ashutosh,
>
> On Mon, Apr 20, 2026 at 10:34 PM Ashutosh Bapat <[email protected]> wrote:
>>
>> On Mon, Apr 20, 2026 at 11:42 PM SATYANARAYANA NARLAPURAM
>> <[email protected]> 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

view thread (13+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: [Bug][patch]: After dropping the last label from a property graph element, invoking pg_get_propgraphdef() triggers an assertion failure
  In-Reply-To: <CAExHW5t2YtTMnK7qbvL++pBet-5B8bzgU+8wMcC5aqZwBH0C9w@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox