Horton
Menu

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Readable identity strings for property graph objects

  2. Handle element label and label property objects in object address functions

  3. Simplify code in objectaddress.c for some property graph objects

  1. Fix DROP PROPERTY GRAPH "unsupported object class" error

    Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2026-04-22T16:19:26Z

    Hi hackers,

While testing the Property Graphs, I observed that DROP PROPERTY GRAPH could
generate the "unsupported object class" error.

Indeed, getObjectTypeDescription() and getObjectIdentityParts() are missing switch
cases for PropgraphElementLabelRelationId and PropgraphLabelPropertyRelationId,
causing DROP PROPERTY GRAPH to hit the default case and error out with
"unsupported object class".

The bug only manifests when an event trigger is active, because that is what
calls these functions.

The attached adds the missing cases so that DROP PROPERTY GRAPH, DROP PROPERTY GRAPH
IF EXISTS, and DROP SCHEMA CASCADE on schemas containing property graphs all work
correctly when event triggers are present.

It also adds test cases that create an event trigger and then exercise DROP PROPERTY
GRAPH and DROP SCHEMA CASCADE with property graphs.

I think that's worth an open item and I'll add one for this issue.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

  2. Re: Fix DROP PROPERTY GRAPH "unsupported object class" error

    Michael Paquier <michael@paquier.xyz> — 2026-04-23T06:39:34Z

    On Wed, Apr 22, 2026 at 04:19:26PM +0000, Bertrand Drouvot wrote:
> Indeed, getObjectTypeDescription() and getObjectIdentityParts() are missing switch
> cases for PropgraphElementLabelRelationId and PropgraphLabelPropertyRelationId,
> causing DROP PROPERTY GRAPH to hit the default case and error out with
> "unsupported object class".

Hmm.  Couldn't these code paths be reached as well with the object
functions like pg_describe_object(), pg_get_object_address(),
pg_identify_object_as_address() or pg_identify_object()?  Object
descriptions usually stick within object_address.sql.  The new objects
you would want to stick should be covered as well in this test suite,
and the file already has some property graphs in it.

> The bug only manifests when an event trigger is active, because that is what
> calls these functions.

--- a/src/test/regress/expected/create_property_graph.out
+++ b/src/test/regress/expected/create_property_graph.out
[...]
+CREATE EVENT TRIGGER dpg_evt ON ddl_command_end EXECUTE FUNCTION dpg_evt_func();

Event triggers are avoided in parallel groups because they are not
reliable (see also fast_default), and we should avoid what you are
doing in this test.

> The attached adds the missing cases so that DROP PROPERTY GRAPH, DROP PROPERTY GRAPH
> IF EXISTS, and DROP SCHEMA CASCADE on schemas containing property graphs all work
> correctly when event triggers are present.
> 
> It also adds test cases that create an event trigger and then exercise DROP PROPERTY
> GRAPH and DROP SCHEMA CASCADE with property graphs.
> 
> I think that's worth an open item and I'll add one for this issue.

This should be an open item, I guess, yes.  Could you add one?  Even
if Peter discards the issue at the end, the issue still needs to be
discussed so we had better to track it anyway.
--
Michael

  3. Re: Fix DROP PROPERTY GRAPH "unsupported object class" error

    Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> — 2026-04-23T07:27:37Z

    On Thu, Apr 23, 2026 at 12:09 PM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Wed, Apr 22, 2026 at 04:19:26PM +0000, Bertrand Drouvot wrote:
> > Indeed, getObjectTypeDescription() and getObjectIdentityParts() are missing switch
> > cases for PropgraphElementLabelRelationId and PropgraphLabelPropertyRelationId,
> > causing DROP PROPERTY GRAPH to hit the default case and error out with
> > "unsupported object class".
>
> Hmm.  Couldn't these code paths be reached as well with the object
> functions like pg_describe_object(), pg_get_object_address(),
> pg_identify_object_as_address() or pg_identify_object()?  Object
> descriptions usually stick within object_address.sql.  The new objects
> you would want to stick should be covered as well in this test suite,
> and the file already has some property graphs in it.

+1. See emails around [1] for some discussion about existing property
graph object definitions.

>
> > The bug only manifests when an event trigger is active, because that is what
> > calls these functions.
>
> --- a/src/test/regress/expected/create_property_graph.out
> +++ b/src/test/regress/expected/create_property_graph.out
> [...]
> +CREATE EVENT TRIGGER dpg_evt ON ddl_command_end EXECUTE FUNCTION dpg_evt_func();
>
> Event triggers are avoided in parallel groups because they are not
> reliable (see also fast_default), and we should avoid what you are
> doing in this test.
>
> > The attached adds the missing cases so that DROP PROPERTY GRAPH, DROP PROPERTY GRAPH
> > IF EXISTS, and DROP SCHEMA CASCADE on schemas containing property graphs all work
> > correctly when event triggers are present.
> >
> > It also adds test cases that create an event trigger and then exercise DROP PROPERTY
> > GRAPH and DROP SCHEMA CASCADE with property graphs.

There are already tests for  DROP PROPERTY GRAPH  and DROP PROPERTY
GRAPH IF EXISTS.

We don't have DROP SCHEMA CASCADE tests in create_table.sql or
create_function.sql. It seems like we don't explicitly test it for
every object separately. Why do we want to add it for property graphs
then?

I could reproduce your issue with a smaller change to the file

diff --git a/src/test/regress/sql/create_property_graph.sql
b/src/test/regress/sql/create_property_graph.sql
index 4cf771596a8..d9cbdf9f1a9 100644
--- a/src/test/regress/sql/create_property_graph.sql
+++ b/src/test/regress/sql/create_property_graph.sql
@@ -151,6 +151,11 @@ CREATE PROPERTY GRAPH gx
         t1x KEY (a) PROPERTIES (b::varchar(20) AS p1),
         t2x KEY (i) PROPERTIES (j::varchar(20) AS p1)  -- matching
typmods by casting works
     );
+CREATE FUNCTION gx_evt_func() RETURNS event_trigger
+LANGUAGE plpgsql AS $$
+BEGIN END;
+$$;
+CREATE EVENT TRIGGER gx_evt ON ddl_command_end EXECUTE FUNCTION gx_evt_func();
 DROP PROPERTY GRAPH gx;
 DROP TABLE t1x, t2x;

That covers everything you want to test and its minimal change to the
test. But I see that the event triggers for all objects are tested in
event_triggers.sql. So I think the new test should be added to that
file.
> >
> > I think that's worth an open item and I'll add one for this issue.
>
> This should be an open item, I guess, yes.  Could you add one?  Even
> if Peter discards the issue at the end, the issue still needs to be
> discussed so we had better to track it anyway.

Element label, label property are not user visible objects per say, so
I am not sure whether the code changes are in the right direction. But
it's also true that we shouldn't get an error in the presence of an
event trigger. We need to fix that.

[1] https://www.postgresql.org/message-id/CAExHW5sgVFHWYkcxZjH0LF4Qbyx2Zyri5ZLave7tZdMbvTLiqg@mail.gmail.com

--
Best Wishes,
Ashutosh Bapat

  4. Re: Fix DROP PROPERTY GRAPH "unsupported object class" error

    Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2026-04-23T11:01:02Z

    Hi,

On Thu, Apr 23, 2026 at 12:57:37PM +0530, Ashutosh Bapat wrote:
> On Thu, Apr 23, 2026 at 12:09 PM Michael Paquier <michael@paquier.xyz> wrote:
> >
> > On Wed, Apr 22, 2026 at 04:19:26PM +0000, Bertrand Drouvot wrote:
> > > Indeed, getObjectTypeDescription() and getObjectIdentityParts() are missing switch
> > > cases for PropgraphElementLabelRelationId and PropgraphLabelPropertyRelationId,
> > > causing DROP PROPERTY GRAPH to hit the default case and error out with
> > > "unsupported object class".
> >
> > Hmm.  Couldn't these code paths be reached as well with the object
> > functions like pg_describe_object(), pg_get_object_address(),
> > pg_identify_object_as_address() or pg_identify_object()?  Object
> > descriptions usually stick within object_address.sql.  The new objects
> > you would want to stick should be covered as well in this test suite,
> > and the file already has some property graphs in it.
> 
> +1. See emails around [1] for some discussion about existing property
> graph object definitions.

Yeah that makes sense to also add some tests here, done in the attached.

> > > The bug only manifests when an event trigger is active, because that is what
> > > calls these functions.
> >
> That covers everything you want to test and its minimal change to the
> test. But I see that the event triggers for all objects are tested in
> event_triggers.sql. So I think the new test should be added to that
> file.

A simpler test has been done in event_trigger.sql instead.

> > > I think that's worth an open item and I'll add one for this issue.
> >
> > This should be an open item, I guess, yes.  Could you add one? 

One has already been created (I think you need to be logged in to see the
updates).

> Element label, label property are not user visible objects per say, so
> I am not sure whether the code changes are in the right direction. But
> it's also true that we shouldn't get an error in the presence of an
> event trigger. We need to fix that.

Agreed.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

  5. Re: Fix DROP PROPERTY GRAPH "unsupported object class" error

    Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> — 2026-04-24T12:07:56Z

    On Thu, Apr 23, 2026 at 4:31 PM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:
>
> Hi,
>
> On Thu, Apr 23, 2026 at 12:57:37PM +0530, Ashutosh Bapat wrote:
> > On Thu, Apr 23, 2026 at 12:09 PM Michael Paquier <michael@paquier.xyz> wrote:
> > >
> > > On Wed, Apr 22, 2026 at 04:19:26PM +0000, Bertrand Drouvot wrote:
> > > > Indeed, getObjectTypeDescription() and getObjectIdentityParts() are missing switch
> > > > cases for PropgraphElementLabelRelationId and PropgraphLabelPropertyRelationId,
> > > > causing DROP PROPERTY GRAPH to hit the default case and error out with
> > > > "unsupported object class".
> > >
> > > Hmm.  Couldn't these code paths be reached as well with the object
> > > functions like pg_describe_object(), pg_get_object_address(),
> > > pg_identify_object_as_address() or pg_identify_object()?  Object
> > > descriptions usually stick within object_address.sql.  The new objects
> > > you would want to stick should be covered as well in this test suite,
> > > and the file already has some property graphs in it.
> >
> > +1. See emails around [1] for some discussion about existing property
> > graph object definitions.
>
> Yeah that makes sense to also add some tests here, done in the attached.
>
> > > > The bug only manifests when an event trigger is active, because that is what
> > > > calls these functions.
> > >
> > That covers everything you want to test and its minimal change to the
> > test. But I see that the event triggers for all objects are tested in
> > event_triggers.sql. So I think the new test should be added to that
> > file.
>
> A simpler test has been done in event_trigger.sql instead.
>
> > > > I think that's worth an open item and I'll add one for this issue.
> > >
> > > This should be an open item, I guess, yes.  Could you add one?
>
> One has already been created (I think you need to be logged in to see the
> updates).
>
> > Element label, label property are not user visible objects per say, so
> > I am not sure whether the code changes are in the right direction. But
> > it's also true that we shouldn't get an error in the presence of an
> > event trigger. We need to fix that.
>
> Agreed.

I was wrong when I said that element label and label property are not
user visible objects. Sorry.

They are very much user visible and can be operated upon separately.
For example ALTER PROPERTY GRAPH ... ALTER VERTEX TABLE ... ALTER
LABEL ... DROP PROPERTIES ... drop label property objects. Similarly
for ALTER PROPERTY GRAPH ... ALTER VERTEX TABLE ... DROP LABEL ... .
So your code changes are needed. However I think the test cases added
in the patch are not sufficient.
1. Earlier in object_address.sql there are instances of property graph
element, property graph property etc. But I don't see property graph
element label and property graph label property there.
2. In create_graph_table.sql there are tests for pg_describe_object(),
pg_identify_object_as_address() and pg_identify_object() for property
graph property, property graph element and property graph label
objects. But I don't see tests added for the objects covered by the
patch.

For create_graph_table.sql I think what we need to do is add a
RECURSIVE CTE like
WITH RECURSIVE deps (classid, objid, objsubid, refclassid, refobjid,
refobjsubid) AS
 (
    SELECT classid, objid, objsubid,
           refclassid, refobjid, refobjsubid
      FROM pg_depend
      WHERE refclassid = 'pg_class'::regclass AND
            refobjid = 'create_property_graph_tests.g2'::regclass

    UNION ALL

    SELECT d.classid, d.objid, d.objsubid,
           d.refclassid, d.refobjid, d.refobjsubid
      FROM pg_depend d
      JOIN deps dp ON d.refclassid = dp.classid AND d.refobjid =
dp.objid AND d.refobjsubid = dp.objsubid
 )
 SELECT pg_describe_object(classid, objid, objsubid) as obj,
       pg_describe_object(refclassid, refobjid, refobjsubid) as reference_graph
    FROM deps
    ORDER BY 1, 2;

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.

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.

-- 
Best Wishes,
Ashutosh Bapat

  6. Re: Fix DROP PROPERTY GRAPH "unsupported object class" error

    Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2026-04-24T14:18:21Z

    Hi,

On Fri, Apr 24, 2026 at 05:37:56PM +0530, Ashutosh Bapat wrote:
> So your code changes are needed. However I think the test cases added
> in the patch are not sufficient.
> 1. Earlier in object_address.sql there are instances of property graph
> element, property graph property etc. But I don't see property graph
> element label and property graph label property there.

I did not add them because they would not produce an error without the
patch in place. That said, that's probably better to add them for consistency,
done in the attached.

> 2. In create_graph_table.sql there are tests for pg_describe_object(),
> pg_identify_object_as_address() and pg_identify_object() for property
> graph property, property graph element and property graph label
> objects. But I don't see tests added for the objects covered by the
> patch.
> 
> For create_graph_table.sql I think what we need to do is add a
> RECURSIVE CTE like
> WITH RECURSIVE deps (classid, objid, objsubid, refclassid, refobjid,
> refobjsubid) AS
>  (
>     SELECT classid, objid, objsubid,
>            refclassid, refobjid, refobjsubid
>       FROM pg_depend
>       WHERE refclassid = 'pg_class'::regclass AND
>             refobjid = 'create_property_graph_tests.g2'::regclass
> 
>     UNION ALL
> 
>     SELECT d.classid, d.objid, d.objsubid,
>            d.refclassid, d.refobjid, d.refobjsubid
>       FROM pg_depend d
>       JOIN deps dp ON d.refclassid = dp.classid AND d.refobjid =
> dp.objid AND d.refobjsubid = dp.objsubid
>  )
>  SELECT pg_describe_object(classid, objid, objsubid) as obj,
>        pg_describe_object(refclassid, refobjid, refobjsubid) as reference_graph
>     FROM deps
>     ORDER BY 1, 2;
> 
> 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 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.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

  7. Re: Fix DROP PROPERTY GRAPH "unsupported object class" error

    Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> — 2026-04-29T12:32:57Z

    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

  8. Re: Fix DROP PROPERTY GRAPH "unsupported object class" error

    Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2026-04-30T11:26:46Z

    Hi,

On Wed, Apr 29, 2026 at 06:02:57PM +0530, Ashutosh Bapat wrote:
> 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.
> >
> > 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.

Works for me.

> 
> 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 can see the the instability locally with en_US.UTF-8 (gt before _gt) and I
can see that, for example sifaka, has en_US.UTF-8 in locales. So, modifying the
impacted query a bit to add COLLATE "C" in the attached.

> 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.

v3 was using the syscan to be consistent with what getObjectDescription() was
doing. That makes sense to me to be consistent, so in passing v5 makes use of
get_catalog_object_by_oid() in getObjectDescription() too.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

  9. Re: Fix DROP PROPERTY GRAPH "unsupported object class" error

    Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> — 2026-04-30T12:31:22Z

    On Thu, Apr 30, 2026 at 4:56 PM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:
>
>
> >
> > 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 can see the the instability locally with en_US.UTF-8 (gt before _gt) and I
> can see that, for example sifaka, has en_US.UTF-8 in locales. So, modifying the
> impacted query a bit to add COLLATE "C" in the attached.
>

I did wonder where the instability is coming from the new query - it's
because now we are traversing the entire dependency tree which also
contains array type of reltype. I think we should just eliminate it
from the result just like we are eliminating the rule. There is slight
convenience in keeping the query as is - it need not change even
though the columns returned by the function change and it's more
readable. I also think that we don't need a type defined for a
property graph - it doesn't contain any row as well as the shape of
the result of GRAPH_TABLE depends upon the COLUMNS clause - so that's
not fixed as well. I will start a separate thread for that discussion.

> > 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.
>
> v3 was using the syscan to be consistent with what getObjectDescription() was
> doing. That makes sense to me to be consistent, so in passing v5 makes use of
> get_catalog_object_by_oid() in getObjectDescription() too.

I was planning to start a separate discussion to change all catalog
lookups in those functions to use get_catalog_object_by_oid(), hence
didn't include changes in getObjectDescription(). But I am fine if we
want to do it in this patch just for the property graph related nodes.

--
Best Wishes,
Ashutosh Bapat

  10. Re: Fix DROP PROPERTY GRAPH "unsupported object class" error

    Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2026-05-05T06:00:23Z

    Hi,

On Thu, Apr 30, 2026 at 06:01:22PM +0530, Ashutosh Bapat wrote:
> On Thu, Apr 30, 2026 at 4:56 PM Bertrand Drouvot
> <bertranddrouvot.pg@gmail.com> wrote:
> >
> > I can see the the instability locally with en_US.UTF-8 (gt before _gt) and I
> > can see that, for example sifaka, has en_US.UTF-8 in locales. So, modifying the
> > impacted query a bit to add COLLATE "C" in the attached.
> >
> 
> I did wonder where the instability is coming from the new query - it's
> because now we are traversing the entire dependency tree which also
> contains array type of reltype.

Right.

> I think we should just eliminate it
> from the result just like we are eliminating the rule. There is slight
> convenience in keeping the query as is - it need not change even
> though the columns returned by the function change and it's more
> readable. I also think that we don't need a type defined for a
> property graph - it doesn't contain any row as well as the shape of
> the result of GRAPH_TABLE depends upon the COLUMNS clause - so that's
> not fixed as well. I will start a separate thread for that discussion.

Yeah, now that 891a57c7394 is in, PFA a new version of the patch. This is just
a mandatory rebase and the COLLATE "C" removals.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

  11. Re: Fix DROP PROPERTY GRAPH "unsupported object class" error

    Alex Guo <guo.alex.hengchen@gmail.com> — 2026-05-06T02:38:32Z

    On 5/5/26 2:00 PM, Bertrand Drouvot wrote:
> Hi,
>
> On Thu, Apr 30, 2026 at 06:01:22PM +0530, Ashutosh Bapat wrote:
>> On Thu, Apr 30, 2026 at 4:56 PM Bertrand Drouvot
>> <bertranddrouvot.pg@gmail.com> wrote:
>>> I can see the the instability locally with en_US.UTF-8 (gt before _gt) and I
>>> can see that, for example sifaka, has en_US.UTF-8 in locales. So, modifying the
>>> impacted query a bit to add COLLATE "C" in the attached.
>>>
>> I did wonder where the instability is coming from the new query - it's
>> because now we are traversing the entire dependency tree which also
>> contains array type of reltype.
> Right.
>
>> I think we should just eliminate it
>> from the result just like we are eliminating the rule. There is slight
>> convenience in keeping the query as is - it need not change even
>> though the columns returned by the function change and it's more
>> readable. I also think that we don't need a type defined for a
>> property graph - it doesn't contain any row as well as the shape of
>> the result of GRAPH_TABLE depends upon the COLUMNS clause - so that's
>> not fixed as well. I will start a separate thread for that discussion.
> Yeah, now that 891a57c7394 is in, PFA a new version of the patch. This is just
> a mandatory rebase and the COLLATE "C" removals.
>
> Regards,

Thanks for the patch. The patch overall LGTM. There is a typo in the commit message:

"manipulated invdividually through ALTER PROPERTY GRAPH sub-commands. Hence they”

invdividually -> individually

Regards,
Alex Guo

  12. Re: Fix DROP PROPERTY GRAPH "unsupported object class" error

    Michael Paquier <michael@paquier.xyz> — 2026-05-06T23:22:49Z

    On Thu, Apr 30, 2026 at 06:01:22PM +0530, Ashutosh Bapat wrote:
> I was planning to start a separate discussion to change all catalog
> lookups in those functions to use get_catalog_object_by_oid(), hence
> didn't include changes in getObjectDescription(). But I am fine if we
> want to do it in this patch just for the property graph related nodes.

It does not strike me as a big deal to use get_catalog_object_by_oid
for PropgraphLabelPropertyRelationId, not does it strike me as a big
deal to keep the code as is as this is new code.  Changing the other
object types to use get_catalog_object_by_oid() is going overboard as
it is not directly related to the issue at hand, so a separate
discussion looks adapted.  I do agree that it would be a good move to
reuse get_catalog_object_by_oid() when we can: a syscache scan (or if
one is added in the future) will be always cheaper than a systable
scan.
--
Michael

  13. Re: Fix DROP PROPERTY GRAPH "unsupported object class" error

    Michael Paquier <michael@paquier.xyz> — 2026-05-07T01:47:32Z

    On Wed, May 06, 2026 at 10:38:32AM +0800, Alex Guo wrote:
> On 5/5/26 2:00 PM, Bertrand Drouvot wrote:
>> On Thu, Apr 30, 2026 at 06:01:22PM +0530, Ashutosh Bapat wrote:
>>> I think we should just eliminate it
>>> from the result just like we are eliminating the rule. There is slight
>>> convenience in keeping the query as is - it need not change even
>>> though the columns returned by the function change and it's more
>>> readable. I also think that we don't need a type defined for a
>>> property graph - it doesn't contain any row as well as the shape of
>>> the result of GRAPH_TABLE depends upon the COLUMNS clause - so that's
>>> not fixed as well. I will start a separate thread for that
>>> discussion.
>>
>> Yeah, now that 891a57c7394 is in, PFA a new version of the patch. This is just
>> a mandatory rebase and the COLLATE "C" removals.

In order to move the needle, I have applied the simplification of
getObjectDescription() as an independent piece.

Label properties lead to part descriptions like that:
+ 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

This is confusing and hard to act on for the reader, with the same
object name defined twice (worse matter: twice in a row).  For the
reader, is the first "e" something different than the second?  Do both
refer to the same object?  Do they refer to different sub-objects
instead but named the same because the implementation dictates so?
This could gain in clarity.

This has been mentioned upthread.  But, while reviewing the rest, I am
really puzzled by your choice of "property graph element label
property" over "property graph label property", which is inconsistent
with the catalog description.  We usually try to be careful about the
wordings of the descriptions with the statis data in objectaddress.c,
and the extra "element" feels out of place to me.

I'll let Peter comment about these points, but it really looks like
the intention of the catalog leads to a result closer to the attached
(leaving the label property description aside for a minute).

Attaching a rebased v7 with the remaining pieces.
--
Michael

  14. Re: Fix DROP PROPERTY GRAPH "unsupported object class" error

    Peter Eisentraut <peter@eisentraut.org> — 2026-05-07T09:00:48Z

    On 07.05.26 03:47, Michael Paquier wrote:
> On Wed, May 06, 2026 at 10:38:32AM +0800, Alex Guo wrote:
>> On 5/5/26 2:00 PM, Bertrand Drouvot wrote:
>>> On Thu, Apr 30, 2026 at 06:01:22PM +0530, Ashutosh Bapat wrote:
>>>> I think we should just eliminate it
>>>> from the result just like we are eliminating the rule. There is slight
>>>> convenience in keeping the query as is - it need not change even
>>>> though the columns returned by the function change and it's more
>>>> readable. I also think that we don't need a type defined for a
>>>> property graph - it doesn't contain any row as well as the shape of
>>>> the result of GRAPH_TABLE depends upon the COLUMNS clause - so that's
>>>> not fixed as well. I will start a separate thread for that
>>>> discussion.
>>>
>>> Yeah, now that 891a57c7394 is in, PFA a new version of the patch. This is just
>>> a mandatory rebase and the COLLATE "C" removals.
> 
> In order to move the needle, I have applied the simplification of
> getObjectDescription() as an independent piece.
> 
> Label properties lead to part descriptions like that:
> + 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
> 
> This is confusing and hard to act on for the reader, with the same
> object name defined twice (worse matter: twice in a row).  For the
> reader, is the first "e" something different than the second?  Do both
> refer to the same object?  Do they refer to different sub-objects
> instead but named the same because the implementation dictates so?
> This could gain in clarity.

Yes, this does not seem very useful.

> This has been mentioned upthread.  But, while reviewing the rest, I am
> really puzzled by your choice of "property graph element label
> property" over "property graph label property", which is inconsistent
> with the catalog description.  We usually try to be careful about the
> wordings of the descriptions with the statis data in objectaddress.c,
> and the extra "element" feels out of place to me.

I think your assessment is correct.  (But you still have the previous 
name in the commit message.)

> I'll let Peter comment about these points, but it really looks like
> the intention of the catalog leads to a result closer to the attached
> (leaving the label property description aside for a minute).
> 
> Attaching a rebased v7 with the remaining pieces.

I had left out these two catalogs from the object address handling 
semi-intentionally.  It's not clear to me what an event trigger would 
want to do with this, or whether they should.  These catalog layouts are 
implementation details, and if we expose this to event triggers, are we 
locked into this catalog layout?  Do we need to document catalog changes 
as breaking user interfaces?

Obviously, we shouldn't leave "unsupported object class" errors lying 
around, but I wonder whether we could also go the other way and 
intentionally skip these catalogs.

  15. Re: Fix DROP PROPERTY GRAPH "unsupported object class" error

    Michael Paquier <michael@paquier.xyz> — 2026-05-07T22:06:52Z

    On Thu, May 07, 2026 at 11:00:48AM +0200, Peter Eisentraut wrote:
> (But you still have the previous name in the commit message.)

(Yes, I did not bother edit the commit message.)

> I had left out these two catalogs from the object address handling
> semi-intentionally.  It's not clear to me what an event trigger would want
> to do with this, or whether they should.  These catalog layouts are
> implementation details, and if we expose this to event triggers, are we
> locked into this catalog layout?  Do we need to document catalog changes as
> breaking user interfaces?
> 
> Obviously, we shouldn't leave "unsupported object class" errors lying
> around, but I wonder whether we could also go the other way and
> intentionally skip these catalogs.

Skipping them feels a bit weird to me as objectaddress.c acts as an
interface to make a bit readable catalog dependencies.  It's true that
we are in a weird spot in this representation due to the way these two
properties are stored in the catalogs, but if we can make the text
presented to the reader less confusing that seems like a benefit to
me.  You have much more context than myself here, of course, so
perhaps my impression is wrong.
--
Michael

  16. Re: Fix DROP PROPERTY GRAPH "unsupported object class" error

    Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> — 2026-06-02T06:57:19Z

    On Thu, May 7, 2026 at 2:30 PM Peter Eisentraut <peter@eisentraut.org> wrote:
>
> On 07.05.26 03:47, Michael Paquier wrote:
> > On Wed, May 06, 2026 at 10:38:32AM +0800, Alex Guo wrote:
> >> On 5/5/26 2:00 PM, Bertrand Drouvot wrote:
> >>> On Thu, Apr 30, 2026 at 06:01:22PM +0530, Ashutosh Bapat wrote:
> >>>> I think we should just eliminate it
> >>>> from the result just like we are eliminating the rule. There is slight
> >>>> convenience in keeping the query as is - it need not change even
> >>>> though the columns returned by the function change and it's more
> >>>> readable. I also think that we don't need a type defined for a
> >>>> property graph - it doesn't contain any row as well as the shape of
> >>>> the result of GRAPH_TABLE depends upon the COLUMNS clause - so that's
> >>>> not fixed as well. I will start a separate thread for that
> >>>> discussion.
> >>>
> >>> Yeah, now that 891a57c7394 is in, PFA a new version of the patch. This is just
> >>> a mandatory rebase and the COLLATE "C" removals.
> >
> > In order to move the needle, I have applied the simplification of
> > getObjectDescription() as an independent piece.
> >
> > Label properties lead to part descriptions like that:
> > + 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
> >
> > This is confusing and hard to act on for the reader, with the same
> > object name defined twice (worse matter: twice in a row).  For the
> > reader, is the first "e" something different than the second?  Do both
> > refer to the same object?  Do they refer to different sub-objects
> > instead but named the same because the implementation dictates so?
> > This could gain in clarity.
>
> Yes, this does not seem very useful.
>
> > This has been mentioned upthread.  But, while reviewing the rest, I am
> > really puzzled by your choice of "property graph element label
> > property" over "property graph label property", which is inconsistent
> > with the catalog description.  We usually try to be careful about the
> > wordings of the descriptions with the statis data in objectaddress.c,
> > and the extra "element" feels out of place to me.
>
> I think your assessment is correct.  (But you still have the previous
> name in the commit message.)

Got it.

>
> > I'll let Peter comment about these points, but it really looks like
> > the intention of the catalog leads to a result closer to the attached
> > (leaving the label property description aside for a minute).
> >
> > Attaching a rebased v7 with the remaining pieces.
>
> I had left out these two catalogs from the object address handling
> semi-intentionally.  It's not clear to me what an event trigger would
> want to do with this, or whether they should.  These catalog layouts are
> implementation details, and if we expose this to event triggers, are we
> locked into this catalog layout?  Do we need to document catalog changes
> as breaking user interfaces?
>
> Obviously, we shouldn't leave "unsupported object class" errors lying
> around, but I wonder whether we could also go the other way and
> intentionally skip these catalogs.
>


The objects in those two catalogs can be manipulated using ALTER
PROPERTY GRAPH ... ALTER VERTEX/EDGE TABLE ... subcommands. If an
event trigger is used to keep copies of the same property graph on
multiple different clusters (logical replicas?) in sync, they will
need to be handed over these objects.

To me your objection seems to apply to the event trigger concept
generally, not specifically for this case. If we change the layout of
any catalog that is supported by getObjectDescriptor(), the event
trigger working on that object may require a change. Isn't that true
already? FormData_pg_* structures depend upon the catalog layout, so
changes to catalog layout may break the user interface. I may be
missing something here.

-- 
Best Wishes,
Ashutosh Bapat

  17. Re: Fix DROP PROPERTY GRAPH "unsupported object class" error

    Michael Paquier <michael@paquier.xyz> — 2026-06-03T02:21:52Z

    On Tue, Jun 02, 2026 at 12:27:19PM +0530, Ashutosh Bapat wrote:
> The objects in those two catalogs can be manipulated using ALTER
> PROPERTY GRAPH ... ALTER VERTEX/EDGE TABLE ... subcommands. If an
> event trigger is used to keep copies of the same property graph on
> multiple different clusters (logical replicas?) in sync, they will
> need to be handed over these objects.

+1.  Even after re-reading the thread again, deciding to skip these
catalogs for the object descriptions still looks like an inconsistent
move compared to all the other catalogs.
--
Michael

  18. Re: Fix DROP PROPERTY GRAPH "unsupported object class" error

    Peter Eisentraut <peter@eisentraut.org> — 2026-06-05T07:10:47Z

    On 07.05.26 03:47, Michael Paquier wrote:
> On Wed, May 06, 2026 at 10:38:32AM +0800, Alex Guo wrote:
>> On 5/5/26 2:00 PM, Bertrand Drouvot wrote:
>>> On Thu, Apr 30, 2026 at 06:01:22PM +0530, Ashutosh Bapat wrote:
>>>> I think we should just eliminate it
>>>> from the result just like we are eliminating the rule. There is slight
>>>> convenience in keeping the query as is - it need not change even
>>>> though the columns returned by the function change and it's more
>>>> readable. I also think that we don't need a type defined for a
>>>> property graph - it doesn't contain any row as well as the shape of
>>>> the result of GRAPH_TABLE depends upon the COLUMNS clause - so that's
>>>> not fixed as well. I will start a separate thread for that
>>>> discussion.
>>>
>>> Yeah, now that 891a57c7394 is in, PFA a new version of the patch. This is just
>>> a mandatory rebase and the COLLATE "C" removals.
> 
> In order to move the needle, I have applied the simplification of
> getObjectDescription() as an independent piece.
> 
> Label properties lead to part descriptions like that:
> + 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
> 
> This is confusing and hard to act on for the reader, with the same
> object name defined twice (worse matter: twice in a row).  For the
> reader, is the first "e" something different than the second?  Do both
> refer to the same object?  Do they refer to different sub-objects
> instead but named the same because the implementation dictates so?
> This could gain in clarity.
> 
> This has been mentioned upthread.  But, while reviewing the rest, I am
> really puzzled by your choice of "property graph element label
> property" over "property graph label property", which is inconsistent
> with the catalog description.  We usually try to be careful about the
> wordings of the descriptions with the statis data in objectaddress.c,
> and the extra "element" feels out of place to me.
> 
> I'll let Peter comment about these points, but it really looks like
> the intention of the catalog leads to a result closer to the attached
> (leaving the label property description aside for a minute).
> 
> Attaching a rebased v7 with the remaining pieces.

I have committed the v7 patch with two additional fixes: 1) Removed the 
translation markers from the getObjectIdentityParts additions, these are 
not supposed to be translated; and 2) added the new cases to ObjectTypeMap.

  19. Re: Fix DROP PROPERTY GRAPH "unsupported object class" error

    Michael Paquier <michael@paquier.xyz> — 2026-06-08T00:19:54Z

    On Fri, Jun 05, 2026 at 09:10:47AM +0200, Peter Eisentraut wrote:
> I have committed the v7 patch with two additional fixes: 1) Removed the
> translation markers from the getObjectIdentityParts additions, these are not
> supposed to be translated; and 2) added the new cases to ObjectTypeMap.

+ property graph element label  |        |      | e of e of
create_property_graph_tests.gt
+ property graph element label  |        |      | v1 of v1 of
create_property_graph_tests.gt
+ property graph element label  |        |      | v2 of v2 of
create_property_graph_tests.gt
[...]
+ property graph label property |        |      | c of e of e of
create_property_graph_tests.gt
+ 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

FWIW, I still find these descriptions written as of "$object of
$object of..", or worse the "$object1 of $object2 of $object2 of..",
really hard to parse, and make some sense out of them.  Am I the only
one?
--
Michael

  20. Re: Fix DROP PROPERTY GRAPH "unsupported object class" error

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-06-08T00:28:40Z

    Michael Paquier <michael@paquier.xyz> writes:
> + property graph element label  |        |      | e of e of
> create_property_graph_tests.gt
> + property graph element label  |        |      | v1 of v1 of
> create_property_graph_tests.gt
> + property graph element label  |        |      | v2 of v2 of
> create_property_graph_tests.gt
> [...]
> + property graph label property |        |      | c of e of e of
> create_property_graph_tests.gt
> + 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

> FWIW, I still find these descriptions written as of "$object of
> $object of..", or worse the "$object1 of $object2 of $object2 of..",
> really hard to parse, and make some sense out of them.  Am I the only
> one?

No.  At the very least, these messages violate our style guidelines [1]:

    Type of the Object 

    When citing the name of an object, state what kind of object it is.

    Rationale: Otherwise no one will know what “foo.bar.baz” refers to.

I'm not sure whether adding that would be sufficient to make these
intelligible, but surely it would help.  I think your first example
would come out like

    label e of property(?) e of property graph create_property_graph_tests.gt

			regards, tom lane

[1] https://www.postgresql.org/docs/current/error-style-guide.html#ERROR-STYLE-GUIDE-OBJECT-TYPE

  21. Re: Fix DROP PROPERTY GRAPH "unsupported object class" error

    Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> — 2026-06-08T04:59:09Z

    On Mon, Jun 8, 2026 at 5:58 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Michael Paquier <michael@paquier.xyz> writes:
> > + property graph element label  |        |      | e of e of
> > create_property_graph_tests.gt
> > + property graph element label  |        |      | v1 of v1 of
> > create_property_graph_tests.gt
> > + property graph element label  |        |      | v2 of v2 of
> > create_property_graph_tests.gt
> > [...]
> > + property graph label property |        |      | c of e of e of
> > create_property_graph_tests.gt
> > + 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
>
> > FWIW, I still find these descriptions written as of "$object of
> > $object of..", or worse the "$object1 of $object2 of $object2 of..",
> > really hard to parse, and make some sense out of them.  Am I the only
> > one?
>
> No.  At the very least, these messages violate our style guidelines [1]:
>
>     Type of the Object
>
>     When citing the name of an object, state what kind of object it is.
>
>     Rationale: Otherwise no one will know what “foo.bar.baz” refers to.
>
> I'm not sure whether adding that would be sufficient to make these
> intelligible, but surely it would help.  I think your first example
> would come out like
>
>     label e of property(?) e of property graph create_property_graph_tests.gt

My guess is Michael and Tom are referring to two different things. I
guess, the output that Michael refers to is the value of the Identity
column from pg_identify_object() (in create_property_graph.out). I
guess what Tom is referring to is the object description in server
error messages, which uses getObjectDescription() underneath, which in
turn is also called from pg_describe_object().
create_property_graph.out has outputs from both pg_describe_object()
and pg_identify_object(). It's easy to get confused between outputs of
both when the outputs are pasted without the query which generated the
output.

pg_describe_object() correctly outputs the description as per the
documented guidelines at [1]. For example "property c of label e of
edge e of property graph gt" or "label e of edge e of property graph
gt". Tom, does that address your concern?

Let's take a look at Michael's concern now. The identity column of
pg_identity_object() ultimately comes from
getObjectIdentity()->getObjectIdentityParts(). The prologue of
getObjectIdentity() says the output is for machine consumption (so not
necessarily human consumable). The identity column should be read in
the context of the object type column of pg_identity_object(). For
example, let's consider the following lines from the output
SELECT (pg_identify_object(classid, objid, objsubid)).*
    FROM (SELECT DISTINCT classid, objid, objsubid FROM deps_tree)
    ORDER BY 1, 2, 3, 4;
             type              | schema | name |
identity
-------------------------------+--------+------+-------------------------------------------------

 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 type of object is "property graph label property", which connects
a property to an element through a label. Hence there is only one
interpretation of the identity column i.e. "property k1/k2 of label e
of element e of property graph gt. That fits the charter of
getObjectIdentityParts(). If we do as Michael suggests we will turn
getObjectIdentityParts() into getObjectDescription(). Those two
functions have different charters  Looking at
getObjectIdentityParts(), it seems that that function should produce
strings which connect objects using prepositions or adjectives instead
of explicit object types. For example, for a constraint
getObjectDescription() returns "constraint C on table R" whereas
getIndentityParts() returns "C on R". For an operator class "operator
class O for access method A" whereas getIdentityParts() returns "O
USING A". Considering these examples I think the current output of
getObjectIdentityParts() and getObjectDescription() for various
property graph components is correct. "of" is the right preposition
connecting components of a property graph. Am I missing something?

[1] https://www.postgresql.org/docs/current/error-style-guide.html#ERROR-STYLE-GUIDE-OBJECT-TYPE

-- 
Best Wishes,
Ashutosh Bapat

  22. Re: Fix DROP PROPERTY GRAPH "unsupported object class" error

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-06-08T18:43:22Z

    Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> writes:
> On Mon, Jun 8, 2026 at 5:58 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> No.  At the very least, these messages violate our style guidelines [1]:

> My guess is Michael and Tom are referring to two different things. I
> guess, the output that Michael refers to is the value of the Identity
> column from pg_identify_object() (in create_property_graph.out). I
> guess what Tom is referring to is the object description in server
> error messages, which uses getObjectDescription() underneath, which in
> turn is also called from pg_describe_object().
> create_property_graph.out has outputs from both pg_describe_object()
> and pg_identify_object(). It's easy to get confused between outputs of
> both when the outputs are pasted without the query which generated the
> output.

Okay, I see that these are coming from pg_identify_object(), while
pg_describe_object() is more verbose.  However, I don't think that
that ends the discussion, because existing cases in pg_identify_object
are not entirely uniform in their succinctness.  You quoted some cases
that support a minimalistic style, but there are others, notably:

case AccessMethodOperatorRelationId produces
	"operator %d (%s, %s) of %s"
while case AccessMethodProcedureRelationId produces
	"function %d (%s, %s) of %s"

case AuthMemRelationId produces
	"membership of role %s in role %s"
(hmm, this one is wrong anyway, since it then translates that string)

case UserMappingRelationId produces
	"%s on server %s"

case PublicationNamespaceRelationId produces
	"%s in publication %s"
as does case PublicationRelRelationId

Each of these has chosen to include an object-type name so that
people won't be totally confused about what's what.  So I think
we're grading on a curve to some extent here, and it certainly
seems to me that these property-graph identifiers are confusing
enough that they deserve more than zero info about which identifier
is what.  I don't think that "C on R" is terribly confusing about
the identity of a constraint, but I totally disagree that
"property graph label property" is sufficient context to disambiguate
"k2 of e of e of create_property_graph_tests.gt".

The argument that these only need to be machine-readable doesn't sway
me a lot.  In the end, any code that is disassembling these strings is
going to be written by a human, and the human is a lot more likely to
make a mistake about which identifier is which if they're not labeled.

			regards, tom lane

  23. Re: Fix DROP PROPERTY GRAPH "unsupported object class" error

    Michael Paquier <michael@paquier.xyz> — 2026-06-10T01:18:40Z

    On Mon, Jun 08, 2026 at 02:43:22PM -0400, Tom Lane wrote:
> The argument that these only need to be machine-readable doesn't sway
> me a lot.  In the end, any code that is disassembling these strings is
> going to be written by a human, and the human is a lot more likely to
> make a mistake about which identifier is which if they're not labeled.

+1.  I'm not a robot yet and I still want to be able to parse these
strings by reading them.  As things stand, this code does not allow
one to understand what each sub-object refers to.  We are still in
beta, let's improve the situation.
--
Michael

  24. Re: Fix DROP PROPERTY GRAPH "unsupported object class" error

    Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> — 2026-06-10T03:58:56Z

    On Wed, Jun 10, 2026 at 6:48 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Mon, Jun 08, 2026 at 02:43:22PM -0400, Tom Lane wrote:
> > The argument that these only need to be machine-readable doesn't sway
> > me a lot.  In the end, any code that is disassembling these strings is
> > going to be written by a human, and the human is a lot more likely to
> > make a mistake about which identifier is which if they're not labeled.
>
> +1.  I'm not a robot yet and I still want to be able to parse these
> strings by reading them.  As things stand, this code does not allow
> one to understand what each sub-object refers to.  We are still in
> beta, let's improve the situation.

Here's a patch fixing it.

-- 
Best Wishes,
Ashutosh Bapat

  25. Re: Fix DROP PROPERTY GRAPH "unsupported object class" error

    Michael Paquier <michael@paquier.xyz> — 2026-06-10T05:19:27Z

    On Wed, Jun 10, 2026 at 09:28:56AM +0530, Ashutosh Bapat wrote:
> Here's a patch fixing it.

Based on the definitions of pglpgid/pgepgid/pgppgid (property graph
relations), pgelelid (property graph element) and plpellabelid
(property graph element label), that looks better.  I am still puzzled
regarding the choice of "label" in the patch for plpellabelid while it
is an "element label" based on the way it is stored in its catalog,
with the catalog matching the object being named "property graph
element label".

There may be a point in suffixing all these objects with a set of
"property graph" strings, but perhaps you are right in limiting the
length of the output without these.  Talking about "element" and
"element label" (not "label"!) would be good enough here.
--
Michael

  26. Re: Fix DROP PROPERTY GRAPH "unsupported object class" error

    Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> — 2026-06-10T11:00:24Z

    On Wed, Jun 10, 2026 at 10:49 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Wed, Jun 10, 2026 at 09:28:56AM +0530, Ashutosh Bapat wrote:
> > Here's a patch fixing it.
>
> Based on the definitions of pglpgid/pgepgid/pgppgid (property graph
> relations), pgelelid (property graph element) and plpellabelid
> (property graph element label), that looks better.  I am still puzzled
> regarding the choice of "label" in the patch for plpellabelid while it
> is an "element label" based on the way it is stored in its catalog,
> with the catalog matching the object being named "property graph
> element label".
>

Which specific string you are referring to?

property graph element label  |        |      | e of element e of
property graph create_property_graph_tests.gt - that doesn't have a
bare label in there

probably the next one
property graph label property |        |      | a of label v1 of
element v1 of property graph create_property_graph_tests.gt

The term "element" comes later "label v1 of element v1".   There's
nothing in the standard called "element label". That term is an
artifact of our implementation. I think "label v1 of element v1" reads
better and follows the standard compared to "element label v1 of
element v1".

There's also a precedent of not using exact catalog names. In case of
user mapping we say " %s of server ...", we don't use "foreign server"
there even though the object is called "foreign server" and the
catalog is pg_foreign_server.

-- 
Best Wishes,
Ashutosh Bapat

  27. Re: Fix DROP PROPERTY GRAPH "unsupported object class" error

    Michael Paquier <michael@paquier.xyz> — 2026-06-17T05:14:48Z

    On Wed, Jun 10, 2026 at 04:30:24PM +0530, Ashutosh Bapat wrote:
> The term "element" comes later "label v1 of element v1".   There's
> nothing in the standard called "element label". That term is an
> artifact of our implementation. I think "label v1 of element v1" reads
> better and follows the standard compared to "element label v1 of
> element v1".

[ ... checks 9075-16-2023 ... ]

The term is "element table label" when referring to a clause, and most
of the places refer to only "label", so I guess that I'm fine with
your wording here.
--
Michael

  28. Re: Fix DROP PROPERTY GRAPH "unsupported object class" error

    Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> — 2026-06-18T05:41:31Z

    On Wed, Jun 17, 2026 at 10:44 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Wed, Jun 10, 2026 at 04:30:24PM +0530, Ashutosh Bapat wrote:
> > The term "element" comes later "label v1 of element v1".   There's
> > nothing in the standard called "element label". That term is an
> > artifact of our implementation. I think "label v1 of element v1" reads
> > better and follows the standard compared to "element label v1 of
> > element v1".
>
> [ ... checks 9075-16-2023 ... ]
>
> The term is "element table label" when referring to a clause, and most
> of the places refer to only "label", so I guess that I'm fine with
> your wording here.
> --
> Michael

Thanks for the confirmation.

Since the original bug, which led to creating a PG 19 open item, is
fixed I am moving the open item to the resolved section.

IIUC, the follow-on discussion seems to have concluded and Michael is
fine with the last version of patch. It feels like we need to commit
that patch. Michael, are you going to commit it or do you expect
somebody else to do it?

-- 
Best Wishes,
Ashutosh Bapat

  29. Re: Fix DROP PROPERTY GRAPH "unsupported object class" error

    Michael Paquier <michael@paquier.xyz> — 2026-06-18T06:06:04Z

    On Thu, Jun 18, 2026 at 11:11:31AM +0530, Ashutosh Bapat wrote:
> IIUC, the follow-on discussion seems to have concluded and Michael is
> fine with the last version of patch. It feels like we need to commit
> that patch. Michael, are you going to commit it or do you expect
> somebody else to do it?

I was waiting for Peter E. to show up as the committer owning this
code, for comments.  Now, I don't mind chiming in if I don't hear back
from him by the beginning of next week.
--
Michael

  30. Re: Fix DROP PROPERTY GRAPH "unsupported object class" error

    Peter Eisentraut <peter@eisentraut.org> — 2026-06-23T07:23:12Z

    On 18.06.26 08:06, Michael Paquier wrote:
> On Thu, Jun 18, 2026 at 11:11:31AM +0530, Ashutosh Bapat wrote:
>> IIUC, the follow-on discussion seems to have concluded and Michael is
>> fine with the last version of patch. It feels like we need to commit
>> that patch. Michael, are you going to commit it or do you expect
>> somebody else to do it?
> 
> I was waiting for Peter E. to show up as the committer owning this
> code, for comments.  Now, I don't mind chiming in if I don't hear back
> from him by the beginning of next week.

I have committed it now.

  31. Re: Fix DROP PROPERTY GRAPH "unsupported object class" error

    Michael Paquier <michael@paquier.xyz> — 2026-06-23T07:25:08Z

    On Tue, Jun 23, 2026 at 09:23:12AM +0200, Peter Eisentraut wrote:
> I have committed it now.

Thanks!
--
Michael