Re: Fix DROP PROPERTY GRAPH "unsupported object class" error - Mailing list pgsql-hackers

On Fri, Apr 24, 2026 at 7:48 PM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:
>

> > for each of the above functions. This query traverses the dependency
> > tree thus covering every object that can appear in a property graph.
> > For  'create_property_graph_tests.g2' the output of the above query
> > 100 rows long which doesn't seem to be worth the code coverage we get.
> > I guess, we need to choose a property graph with a smaller dependency
> > tree like gt. I haven't examined whether that graph would cover all
> > the cases, but it will certainly cover all the objects.
>
> Yeah, gt is enough to cover all the objects. v3 attached makes use of it and
> 1/ get rid of the "reference_graph" as it's not needed for the test and 2/
> use COLLATE "C" in the order by to be on the safe side of things.

I was thinking of modifying the existing queries as attached.

I think COLLATE "C" is safer, however, it doesn't go well with indexes
in ORDER BY, which make this query easy to write. (see [1]). So far we
haven't seen any buildfarm failures so far with the current ORDER BY.
That makes me think that the query output will be stable even without
COLLATE "C". So keeping it that way. But we can add COLLATE "C" if we
see buildfarm instability.

>
> > I think the proper description of property graph label property object
> > is property graph element label property since we are reporting
> > property of an element through a label. But then that means the
> > description would be inconsistent with the catalog name and adding
> > "element" to the catalog name would make it much longer. I am not able
> > to decide whether to add "element" in the description or not, ATM.
>
> I think it's better to be consistent with the current catalog here to stay
> focused on the main purpose of this patch.
>

Though getObjectTypeDescription() usually prints the description which
is closer to the name of the catalog, that's not always true. E.g
AccessMethodProcedureRelationId maps to "function of access method",
StatisticExtRelationId maps to "statistics object". I think "property
graph element label property" is more appropriate for
PropgraphLabelPropertyRelationId since the property is associated with
the label which in turn is associated with the element. Attached patch
has that change. If Peter feels "property graph label property" is
better, we will use that.

Some more changes as listed below
1. In event_trigger.sql I have modified the names of the tables a bit
so that I can also add an edge element and then test ALTER PROPERTY
GRAPH as well.
2. Used get_catalog_object_by_oid(), which appropriately uses the
cache or table scan in getObjectIdentityParts(). It performs an extra
lookup but I think that's ok, since the latter function is not in a
hot path. I wonder whether get_catalog_object_by_oid() is intended to
be called whenever we want to get the catalog tuple by OID instead of
directly calling sys cache lookup function or catalog scan. At least
the new code you are adding can make use of this function.
3. The cases in getObjectTypeDescription() and
getObjectIdentityParts() are arranged roughly in alphabetical order,
but not exactly. I took some liberty to rearrange the property graph
related cases such that the cases directly dependent on property graph
appear first in the alphabetical order followed by those dependent
through one hop and then those through two hops. That way the code
appears in the order of their dependency and is easy to read. Also
arranged the new entries in objectaddress.sql to follow that order as
per a comment in that file. The way you had arranged it appeared in
alphabetical order if we considered RelationId to be part of the name
not otherwise.
4. getObjectIdentityParts() should be closer to getObjectDescription()
when creating the array of object names and also the identity. For
example getObjectIdentityParts() constructs identity of the property
as "a of create_property_graph_tests.gt" losing the name of the label
whereas getObjectDescription() constructs it as property "a of label
v1 of vertex v1 of property graph gt". Fixed that as well.

[1] https://www.postgresql.org/message-id/2173dc58-6697-1e10-9604-a7c9f2a8bb55@lab.ntt.co.jp

--
Best Wishes,
Ashutosh Bapat