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.
