Re: Fix DROP PROPERTY GRAPH "unsupported object class" error
Peter Eisentraut <peter@eisentraut.org>
From: Peter Eisentraut <peter@eisentraut.org>
To: Michael Paquier <michael@paquier.xyz>,
Alex Guo <guo.alex.hengchen@gmail.com>
Cc: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>,
Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>,
pgsql-hackers@lists.postgresql.org
Date: 2026-06-05T07:10:47Z
Lists: pgsql-hackers
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.
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Readable identity strings for property graph objects
- 2a7e95b659df 19 (unreleased) landed
-
Handle element label and label property objects in object address functions
- 72498a86989e 19 (unreleased) landed
-
Simplify code in objectaddress.c for some property graph objects
- 6827de95ee09 19 (unreleased) landed