Hi, On Mon, Apr 20, 2026 at 11:58 PM Ashutosh Bapat < [email protected]> wrote:
> 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. > Addressed this in v3 patch. Thanks, Satya
v3-0001-Prevent-dropping-the-last-label-from-a-property-grap.patch
Description: Binary data
