| From: | SATYANARAYANA NARLAPURAM <satyanarlapuram(at)gmail(dot)com> |
|---|---|
| To: | Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> |
| Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: [Bug][patch]: After dropping the last label from a property graph element, invoking pg_get_propgraphdef() triggers an assertion failure |
| Date: | 2026-04-21 05:58:40 |
| Message-ID: | CAHg+QDcXxmeiuySsyR5K=8Wk9Pq2sZ_4HNb6EMonN=yjLz_fyw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
HI Ashutosh,
On Mon, Apr 20, 2026 at 10:34 PM Ashutosh Bapat <
ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
> On Mon, Apr 20, 2026 at 11:42 PM SATYANARAYANA NARLAPURAM
> <satyanarlapuram(at)gmail(dot)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
| Attachment | Content-Type | Size |
|---|---|---|
| v2-0001-Prevent-dropping-the-last-label-from-a-property-grap.patch | application/octet-stream | 4.4 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Richard Guo | 2026-04-21 06:05:07 | Re: Bug: Rule actions see wrong values for generated columns (NEW.gen reads OLD value) |
| Previous Message | Michael Paquier | 2026-04-21 05:55:45 | Re: [PATCH] Prevent repeated deadlock-check signals in standby buffer pin waits |