On Thu, Apr 30, 2026 at 4:56 PM Bertrand Drouvot <[email protected]> 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
