Feedback-ID: ie0a040ee:Fastmail
Message-ID: <29c33fb6-4200-4a08-be3d-a1a2ceef23b8@eisentraut.org>
Date: Fri, 5 Jun 2026 09:10:47 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Fix DROP PROPERTY GRAPH "unsupported object class" error
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
References: <aem-pjK57peuLI-x@paquier.xyz>
 <CAExHW5vtRyD_kfFP5S6o=+afzpR7Yd7sS=cia_0uHhM3GzLcag@mail.gmail.com>
 <aen77u5go8P7RJaL@bdtpg>
 <CAExHW5vfxVCQFADhiEmi3goyOgGb5qQjURHaM2QDtW6CksUzWA@mail.gmail.com>
 <aet7rawq/V07+H1L@bdtpg>
 <CAExHW5vJGHDXyLDAVC3eJF_ykU73AB9CG6HvJ+WtmO+GE4dnzA@mail.gmail.com>
 <afM8dgqqxsxqmnY7@bdtpg>
 <CAExHW5udMZbMKiQFWB=hDeAK+=t2PXPrmBuscpUc3888kzAi2A@mail.gmail.com>
 <afmHd5vSCJtMFueS@bdtpg> <c948abd3-7483-473c-a0e7-29c579c823fc@gmail.com>
 <afvvNP8b6QEfCNsQ@paquier.xyz>
Content-Language: en-US
From: Peter Eisentraut <peter@eisentraut.org>
In-Reply-To: <afvvNP8b6QEfCNsQ@paquier.xyz>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/29c33fb6-4200-4a08-be3d-a1a2ceef23b8%40eisentraut.org>
Precedence: bulk

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.