Fix DROP PROPERTY GRAPH "unsupported object class" error

First seen: 2026-04-22 16:19:26+00:00 · Messages: 13 · Participants: 4

Latest Update

2026-05-07 · opus 4.7

What's New in This Round

Michael Paquier has moved from narrow test-placement feedback into the substantive naming/identity debate, and has committed a partial fix while pushing back on two points that were considered settled in the prior round.

1. Partial commit landed (split-off)

Michael applied the getObjectDescription() simplification (the get_catalog_object_by_oid() conversion for that function) as an independent commit, decoupling it from the remaining bug fix. He explicitly endorses the "use get_catalog_object_by_oid() where we can" direction (cheaper than systable scans, and cheaper still if a syscache is later added), but considers a sweeping conversion of all object types out of scope — agreeing with Ashutosh that it belongs in a separate thread.

2. Michael re-opens the "element label property" naming decision

This is the notable position shift. In the prior round Ashutosh's argument for "property graph element label property" (mirroring the element→label→property hierarchy rather than the catalog name PropgraphLabelProperty) had effectively won, with the group deferring to Peter. Michael now pushes back explicitly:

He defers the final call to Peter but has posted a v7 that reflects his preferred direction (catalog-aligned naming), leaving the label-property description itself unresolved pending Peter's input.

3. New concrete objection: ambiguous identity string "e of e of"

Michael surfaces a readability problem in the identity output that the prior round did not discuss. Sample output:

property graph label property | k1 of e of e of create_property_graph_tests.gt
property graph label property | k2 of e of e of create_property_graph_tests.gt

The repeated "e of e of" occurs because in the test schema the element name and the label name are both e (the implicit label derived from the element table). To a reader this is genuinely ambiguous: same token appearing twice consecutively gives no hint whether they refer to the same entity, nested sub-objects, or coincidentally same-named distinct objects. This is a direct critique of the identity format that the prior round landed on (which Ashutosh had pushed to include both element and label names for round-trip correctness).

Note the tension: the prior analysis framed "include the label name in the identity" as a correctness fix (identity must uniquely identify, else pg_get_object_address can't round-trip). Michael doesn't dispute correctness — he's complaining about the presentation of that correct-but-confusing string. A resolution would likely need to disambiguate with role keywords (e.g., "k1 of label e of element e of ...") rather than bare of chains. This isn't resolved in v7.

4. v7 posted

Rebased v7 contains "the remaining pieces" after the getObjectDescription() split-off commit. Per Michael's description it leans toward catalog-aligned naming ("property graph label property") but leaves the label-property description open for Peter's verdict.

Net State

The patch is no longer a single unit: the uncontroversial refactor has been committed, and what remains is blocked on two Peter-decisions: (a) should the description carry the extra "element" word, and (b) how to make the identity string unambiguous when element and label names collide. Ashutosh's earlier design choices are partially under review again.

History (1 prior analysis)
2026-05-06 · opus 4.7

Analysis: Fix DROP PROPERTY GRAPH "unsupported object class" error

Core Problem

PostgreSQL's SQL/PGQ Property Graph feature (a major new capability being integrated, with Peter Eisentraut as the primary author) introduces several new catalogs to represent the decomposed structure of a property graph:

  • pg_propgraph_element — vertex/edge element tables
  • pg_propgraph_element_label (PropgraphElementLabelRelationId) — labels attached to elements
  • pg_propgraph_label_property (PropgraphLabelPropertyRelationId) — properties projected through a label

These catalogs participate in the standard pg_depend dependency graph, which means the generic object-address machinery in src/backend/catalog/objectaddress.c must know how to describe and identify each catalog's rows. Two central dispatch functions — getObjectTypeDescription() and getObjectIdentityParts() — contain a large switch over classId (the catalog OID). Any catalog class that is missing from these switches falls through to the default arm, which raises ERROR: unsupported object class %u.

The initial Property Graph commit wired up the two new dependent catalogs into pg_depend but never added the corresponding switch arms. Under normal operation this is latent: DROP PROPERTY GRAPH simply walks dependencies and deletes rows without needing textual descriptions. The bug surfaces only when an event trigger is active on ddl_command_end (or related events): the event-trigger support code calls pg_event_trigger_dropped_objects() which in turn invokes getObjectTypeDescription()/getObjectIdentityParts() for every dropped sub-object to build the reported tuple. At that point the two missing classes trigger the default case and abort the DROP.

Architecturally this is the classic "extend a catalog without extending the object-address registry" bug — the same failure mode that has historically hit new object kinds (statistics objects, publication rels, etc.). It matters because the object-address API is also the foundation of pg_describe_object(), pg_identify_object(), pg_identify_object_as_address(), and pg_get_object_address(), so the hole is broader than just event triggers.

The Fix

The patch adds switch arms for PropgraphElementLabelRelationId and PropgraphLabelPropertyRelationId in:

  1. getObjectTypeDescription() — returns the human-readable type string. Ashutosh argued (and ultimately won the point) that PropgraphLabelPropertyRelationId should be described as "property graph element label property" rather than plain "property graph label property", because the property is semantically reached via element → label → property. This breaks the usual convention of keeping the description close to the catalog name (StatisticExtRelationId → "statistics object" is cited as precedent that the convention is not absolute).

  2. getObjectIdentityParts() — produces the fully-qualified identity used by pg_get_object_address() round-tripping. A significant correctness point Ashutosh raised: the initial patch's identity was lossy. For a label property it produced "a of create_property_graph_tests.gt", omitting the label name, whereas getObjectDescription() correctly produced property "a of label v1 of vertex v1 of property graph gt". Identity strings must uniquely identify the object, so the label (and element) names must be included. The committed version aligns the identity construction with the description.

  3. pg_get_object_address / ObjectPropertyType table entries in objectaddress.c, reordered to follow dependency depth rather than strict alphabetical order (direct dependents of property graph first, then one-hop, then two-hop). This is a readability convention Ashutosh justified by a comment already present in the file.

  4. Use of get_catalog_object_by_oid() instead of open-coded syscache/catalog scans. This helper transparently selects syscache vs. seqscan and is the preferred idiom in modern object-address code. The final version propagates this into getObjectDescription() as well for consistency, though Ashutosh flagged a broader cleanup (converting all such lookups) as a separate future thread.

Testing Strategy — the real design debate

The testing discussion was the most substantive part of the thread.

Where to test? The initial patch added an event trigger inside create_property_graph.sql. Michael Paquier objected on two grounds:

  • Event triggers are deliberately kept out of parallel regression groups because they are globally visible and unreliable under concurrency (he cited the fast_default precedent).
  • Object-description tests traditionally live in object_address.sql, which already exercises property graphs.

The resolution moved the event-trigger-based test into event_trigger.sql (which already runs in an isolated group) and added pg_describe_object / pg_identify_object / pg_identify_object_as_address coverage in create_graph_table.sql.

How to achieve coverage without fragility? Ashutosh proposed a recursive CTE over pg_depend rooted at the property graph's pg_class entry, which naturally walks every dependent sub-object (elements, labels, label-properties, properties). Applied to g2 this produced ~100 rows — too much test churn — so the smaller gt graph was chosen as the root, giving full class coverage with a manageable expected output.

Ordering stability. The recursive walk introduced a locale-sensitivity: under en_US.UTF-8 (as on buildfarm member sifaka) gt sorts before _gt (the array type autogenerated for the property graph's composite rowtype), but under C the underscore sorts first. Bertrand added COLLATE "C" to stabilize. Ashutosh then observed that the real issue is that property graphs shouldn't be generating an array type at all — a property graph has no rows and GRAPH_TABLE's output shape depends on the COLUMNS clause — and opened a separate thread to remove it. Once commit 891a57c7394 landed (removing the spurious rowtype/array type), the COLLATE "C" could be dropped.

Key Technical Insights

  1. The object-address registry is a recurring source of silent bugs. Any new catalog carrying pg_depend rows must be added to four places: getObjectTypeDescription, getObjectIdentityParts, getObjectDescription, and the ObjectProperty[] table in objectaddress.c. Missing arms are not caught by normal DROP paths, only by event triggers and the pg_identify_object* family.

  2. Identity vs. description asymmetry was a real correctness bug, not cosmetic. If getObjectIdentityParts() drops a qualifying name (the label), then pg_get_object_address(pg_identify_object_as_address(...)) cannot round-trip, which breaks logical replication of DDL and any tooling that serializes object references.

  3. The spurious composite/array type for property graphs (uncovered incidentally during test stabilization) is a deeper design wart: property graphs are not row sources, so they shouldn't reuse pg_class's rowtype machinery. This spawned follow-up work (commit 891a57c7394).

  4. get_catalog_object_by_oid() is the preferred modern idiom for OID-to-tuple lookup in object-address code because it hides the syscache/seqscan decision. The thread noted that many older sites still open-code this and could benefit from mechanical conversion.

  5. Description text conventions are soft. The decision to say "property graph element label property" rather than mirroring the catalog name PropgraphLabelProperty reflects that descriptions should model the user's mental hierarchy, not the catalog's storage layout.

Participant Dynamics

  • Peter Eisentraut (not on-thread but repeatedly invoked) is the property graph feature author; both Bertrand and Ashutosh deferred to his ultimate judgment on naming ("If Peter feels 'property graph label property' is better, we will use that").
  • Michael Paquier (committer) intervened narrowly on test placement/parallelism hygiene — his concern about event triggers in parallel groups is a well-established project rule and was accepted immediately.
  • Ashutosh Bapat (committer, domain expertise in property graphs from prior review rounds) drove most of the design refinements: broader test coverage, identity-string correctness, dependency-ordered code layout, and catching the spurious array-type issue. His stance effectively steered the final shape of the patch.
  • Bertrand Drouvot (patch author) was responsive and iterated quickly through v1–v6.
  • Alex Guo provided a final typo catch on the commit message.

This is a textbook example of a "small bug-fix patch" that expanded in scope under review to fix adjacent correctness issues (identity round-tripping) and triggered a separate follow-up (removing property graph rowtypes).