| From: | Michael Paquier <michael(at)paquier(dot)xyz> |
|---|---|
| To: | Alex Guo <guo(dot)alex(dot)hengchen(at)gmail(dot)com> |
| Cc: | Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(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-05-07 01:47:32 |
| Message-ID: | afvvNP8b6QEfCNsQ@paquier.xyz |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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
| Attachment | Content-Type | Size |
|---|---|---|
| v7-0001-Handle-element-label-and-element-label-property-o.patch | text/plain | 28.6 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Junwang Zhao | 2026-05-07 02:47:20 | Re: Question: Should we release the FK fast-path pk_slot's buffer pin promptly? |
| Previous Message | Henson Choi | 2026-05-07 01:38:35 | Re: CREATE TABLE LIKE INCLUDING TRIGGERS |