| From: | Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> |
|---|---|
| To: | Michael Paquier <michael(at)paquier(dot)xyz> |
| Cc: | Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Peter Eisentraut <peter(at)eisentraut(dot)org> |
| Subject: | Re: Fix DROP PROPERTY GRAPH "unsupported object class" error |
| Date: | 2026-04-23 07:27:37 |
| Message-ID: | CAExHW5vtRyD_kfFP5S6o=+afzpR7Yd7sS=cia_0uHhM3GzLcag@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Thu, Apr 23, 2026 at 12:09 PM Michael Paquier <michael(at)paquier(dot)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.
--
Best Wishes,
Ashutosh Bapat
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Ashutosh Bapat | 2026-04-23 07:32:47 | Re: [Patch] Block ALTER TABLE RENAME COLUMN when column is used by property graph |
| Previous Message | Peter Smith | 2026-04-23 07:24:49 | Re: Redundant/mis-use of _(x) gettext macro? |