On Thu, Apr 30, 2026 at 4:56 PM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:
>
>
> >
> > I think COLLATE "C" is safer, however, it doesn't go well with indexes
> > in ORDER BY, which make this query easy to write. (see [1]). So far we
> > haven't seen any buildfarm failures so far with the current ORDER BY.
> > That makes me think that the query output will be stable even without
> > COLLATE "C". So keeping it that way. But we can add COLLATE "C" if we
> > see buildfarm instability.
>
> I can see the the instability locally with en_US.UTF-8 (gt before _gt) and I
> can see that, for example sifaka, has en_US.UTF-8 in locales. So, modifying the
> impacted query a bit to add COLLATE "C" in the attached.
>
I did wonder where the instability is coming from the new query - it's
because now we are traversing the entire dependency tree which also
contains array type of reltype. 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.
> > 2. Used get_catalog_object_by_oid(), which appropriately uses the
> > cache or table scan in getObjectIdentityParts(). It performs an extra
> > lookup but I think that's ok, since the latter function is not in a
> > hot path. I wonder whether get_catalog_object_by_oid() is intended to
> > be called whenever we want to get the catalog tuple by OID instead of
> > directly calling sys cache lookup function or catalog scan. At least
> > the new code you are adding can make use of this function.
>
> v3 was using the syscan to be consistent with what getObjectDescription() was
> doing. That makes sense to me to be consistent, so in passing v5 makes use of
> get_catalog_object_by_oid() in getObjectDescription() too.
I was planning to start a separate discussion to change all catalog
lookups in those functions to use get_catalog_object_by_oid(), hence
didn't include changes in getObjectDescription(). But I am fine if we
want to do it in this patch just for the property graph related nodes.
--
Best Wishes,
Ashutosh Bapat
