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

2026-05-06 · opus 4.7

Core Problem

PostgreSQL's SQL/PGQ property graph feature (introduced as part of the ongoing SQL:2023 property graph query support) models graph elements (vertex and edge tables) that each carry one or more labels. Labels are stored in the catalog pg_propgraph_element_label and are dereferenced by pg_get_propgraphdef() in ruleutils.c when reconstructing the DDL of a property graph.

The bug: ALTER PROPERTY GRAPH ... ALTER ... DROP LABEL allows removing labels down to zero. Once an element has no labels, pg_get_propgraphdef() trips Assert(count > 0) at ruleutils.c:1837 — the ruleutils reconstruction code assumes every element has at least one label (which is a reasonable invariant because a label-less graph element cannot be queried meaningfully and cannot be re-created via DDL, since CREATE PROPERTY GRAPH syntax demands at least one label per element).

This is not merely a missing assert guard; it represents a catalog-state invariant violation. Allowing zero labels produces a property graph that cannot be round-tripped through pg_dump (which relies on pg_get_propgraphdef). So the correct fix is at the DDL level, not at the ruleutils level.

Standards Grounding

Ashutosh Bapat anchored the fix in SQL/PGQ standard §11.25 syntax rule 6:

"Element table descriptor shall include two or more labels, one of which has an that is equivalent to the simply contained in the ."

This rule mandates that DROP LABEL is legal only when the element currently has ≥2 labels — i.e., the last label cannot be dropped. PostgreSQL's fix therefore matches the standard rather than being a PG-specific invention. Interestingly, Ashutosh also observed that DROP PROPERTIES has no analogous restriction — dropping all properties is permitted — so the fix must be label-specific, not generalized.

Design Decision: Prevent vs. Handle Zero Labels

Two options were considered:

Prevent dropping the last label (raise an error in AlterPropGraph)
Handle zero-label elements gracefully in ruleutils and downstream consumers

Option 1 was chosen on two grounds: (a) it aligns with the SQL/PGQ standard, and (b) it is strictly simpler than auditing every code path (pg_get_propgraphdef, the planner's graph expansion, pg_dump) for zero-label safety. Handling zero labels would require choosing a DDL representation for an empty label list — which the standard does not define — and would bake a non-standard state into the catalog.

Patch Evolution

v1 (Satya): Added a catalog scan in AlterPropGraph() on pg_propgraph_element_label before the performDeletion() call, counting labels for the element and erroring if only one remained. Included a regression test.

v2: Addressed Ashutosh's review — instead of counting all labels (O(n) scan to the end), break early once a second label is seen. This is the correct optimization: we only need existence of "at least one other label," not the total count.

v3: Added documentation updates noting the last-label restriction.

v4 (Ashutosh's edits):

Changed errcode from ERRCODE_INVALID_OBJECT_DEFINITION to ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE. This is a meaningful semantic distinction: INVALID_OBJECT_DEFINITION implies the label or element is itself malformed, whereas the actual condition is that the element is in a state (single label remaining) that is not a prerequisite for the requested operation. This choice follows the precedent of other PG DDL where SQLSTATE 55000 is used for "cannot perform operation because object is in wrong state."
Considered but rejected factoring the check into a helper function. The rejection reasoning is worth noting: both candidate abstractions (HasMoreThanTwoLabels() or HasAnotherLabelBesides(oid)) leak the drop-label context into the helper's API, providing no real reuse. This is a sound judgment — premature abstraction for a 10-line check would obscure rather than clarify.

v5 (Ashutosh, 2026-04-28): Consolidated the two separate catalog lookups (one to find the target label, one to count siblings) into a single systable_beginscan loop. This is a meaningful performance refinement: pg_propgraph_element_label is scanned on pgelid (element OID), so walking it once and both identifying the victim tuple and checking for siblings is cleaner than two index scans.

Open Issues at Thread End

Peter Eisentraut raised two concerns on 2026-05-04 that were not resolved in the visible thread:

Variable naming: After Ashutosh's consolidation, the local variables elrel and elscan now refer to pg_propgraph_element_label but their names suggest pg_propgraph_element. Peter suggested ellabelrel. This is a minor stylistic fix but important for readability in a subsystem with two closely-named catalogs.
Concurrency / locking: This is the substantive unresolved issue. The check-then-delete pattern:
```
scan for siblings → if none, error; else performDeletion()
```
is racy. Two concurrent DROP LABEL sessions could each see two labels, each conclude the other is the "sibling," and both proceed — resulting in zero labels. The fix likely requires taking a stronger lock on the property graph element row (e.g., ShareUpdateExclusiveLock or row-level lock via LockTuple/SELECT FOR UPDATE semantics on the element row) before the sibling check. The thread ends before this is patched.

Side Discussion: OidIsValid Style

Evan Li posted a 0003 patch normalizing !labeloid and pgrelid == InvalidOid to OidIsValid() / !OidIsValid(). Peter pushed back: he personally doesn't subscribe to that style rule and, more importantly, argued against churn during code stabilization. Evan conceded, noting the existing propgraphcmds.c already uses all three forms inconsistently. This is a stabilization-phase judgment call: style consistency patches trade review bandwidth against code quality, and Peter as the likely committer of the property graph feature has priority-setting authority here.

Architectural Implications

The bug exemplifies a recurring pattern in newly-landed features: catalog invariants that the read path (ruleutils, planner) assumes are not uniformly enforced by the write path (DDL). Similar historical bugs exist around partitioned tables, publications, and extensions. The proper defense is to enforce the invariant at DDL time (as this patch does) rather than relax the reader's assumption — because the reader's assumption typically encodes a real semantic property (here: a graph element must be identifiable by at least one label).

The concurrency gap Peter flagged is also characteristic of new DDL: ALTER PROPERTY GRAPH appears to be using standard performDeletion() dependency machinery without an explicit element-level lock that would serialize label modifications. Resolving this likely requires either elevating the lock level on the property graph relation during ALTER, or introducing tuple-level locking on the element row in pg_propgraph_element.