On Tue, Jun 2, 2026 at 10:13 AM Kyotaro Horiguchi <[email protected]> wrote: > > Hello. Thank you for the comments. > > At Mon, 1 Jun 2026 17:18:13 +0530, Ashutosh Bapat > <[email protected]> wrote in > > 1. There are other places in the function where we use similar code. > > Those places call initStringInfo just before getRelationDescription() > > is called and then pfree() StringInfoData.data after it is added to > > the object description. > > That's a good point. I hadn't paid much attention to that because the > allocation is short-lived, but matching the surrounding code is > probably better. I've added pfree() in the attached patch. > > > 2. Why do we want to capture the output of getObjectDescription() in a > > StringInfo and then add it to the buffer? I think we can pass > > getObjectDescription directly as an argument to appendStringInfo() > > similar to the code for case AttrDefaultRelationId. > > I considered returning strings from helper functions such as > getRelationDescription(), but that would affect a number of similar > helper functions and broaden the scope of the patch. > > While working on this, I noticed that the string returned by > getObjectDescription() in the AttrDefaultRelationId case is not > pfree'd. Since that appears unrelated to this patch, I left it as-is. > > I also dropped the assertions on the temporary description strings. > They were mainly there while developing the patch and don't seem > necessary in the final version.
Thanks. I like the idea of pfree'ing the string returned by getObjectDescriptor() since we also pffree other descriptor strings. But just doing it for these changes doesn't look consistent. Attached patchset with some more changes to bring the new code inline with the surrounding code. The changes are 1. rename objdesc to rel. The latter is used in the surrounding code. 2. getObjectDescriptor() is invoked directly from appendStringInfo. 3. Place InitStringInfo() closer to the appendStringInfo call like surrounding code None of these changes are compulsory. If you think any of the changes are acceptable to you, please provide a single patch absorbing them into your patch. Otherwise, let the committer decide. 0008 is your patch as is 0009 is my cosmetic changes The numbering starts from 0008 since I create all SQL/PGQ patches for various fixes from the same branch. Attached patches are applicable independently. -- Best Wishes, Ashutosh Bapat
From e12e3988856e22a889b518097bf5ae125a169114 Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat <[email protected]> Date: Tue, 2 Jun 2026 11:34:58 +0530 Subject: [PATCH v20260602 9/9] Cosmetic adjustments ... to make the code look consistent with the surrounding code. Author: Ashutosh Bapat <[email protected]> --- src/backend/catalog/objectaddress.c | 52 +++++++++++++---------------- 1 file changed, 23 insertions(+), 29 deletions(-) diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c index 7b0d163cec9..a70f17cc52f 100644 --- a/src/backend/catalog/objectaddress.c +++ b/src/backend/catalog/objectaddress.c @@ -4077,9 +4077,7 @@ getObjectDescription(const ObjectAddress *object, bool missing_ok) { HeapTuple tup; Form_pg_propgraph_element pgeform; - StringInfoData objdesc; - - initStringInfo(&objdesc); + StringInfoData rel; tup = SearchSysCache1(PROPGRAPHELOID, ObjectIdGetDatum(object->objectId)); if (!HeapTupleIsValid(tup)) @@ -4092,16 +4090,17 @@ getObjectDescription(const ObjectAddress *object, bool missing_ok) pgeform = (Form_pg_propgraph_element) GETSTRUCT(tup); - getRelationDescription(&objdesc, pgeform->pgepgid, false); + initStringInfo(&rel); + getRelationDescription(&rel, pgeform->pgepgid, false); if (pgeform->pgekind == PGEKIND_VERTEX) - appendStringInfo(&buffer, _("vertex %s of %s"), NameStr(pgeform->pgealias), objdesc.data); + appendStringInfo(&buffer, _("vertex %s of %s"), NameStr(pgeform->pgealias), rel.data); else if (pgeform->pgekind == PGEKIND_EDGE) - appendStringInfo(&buffer, _("edge %s of %s"), NameStr(pgeform->pgealias), objdesc.data); + appendStringInfo(&buffer, _("edge %s of %s"), NameStr(pgeform->pgealias), rel.data); else - appendStringInfo(&buffer, "??? element %s of %s", NameStr(pgeform->pgealias), objdesc.data); + appendStringInfo(&buffer, "??? element %s of %s", NameStr(pgeform->pgealias), rel.data); - pfree(objdesc.data); + pfree(rel.data); ReleaseSysCache(tup); break; } @@ -4112,7 +4111,6 @@ getObjectDescription(const ObjectAddress *object, bool missing_ok) HeapTuple tuple; Form_pg_propgraph_element_label pgelform; ObjectAddress oa; - char *objdesc; rel = table_open(PropgraphElementLabelRelationId, AccessShareLock); tuple = get_catalog_object_by_oid(rel, @@ -4131,11 +4129,10 @@ getObjectDescription(const ObjectAddress *object, bool missing_ok) ObjectAddressSet(oa, PropgraphElementRelationId, pgelform->pgelelid); - objdesc = getObjectDescription(&oa, false); + appendStringInfo(&buffer, _("label %s of %s"), + get_propgraph_label_name(pgelform->pgellabelid), + getObjectDescription(&oa, false)); - appendStringInfo(&buffer, _("label %s of %s"), get_propgraph_label_name(pgelform->pgellabelid), objdesc); - - pfree(objdesc); table_close(rel, AccessShareLock); break; } @@ -4144,9 +4141,7 @@ getObjectDescription(const ObjectAddress *object, bool missing_ok) { HeapTuple tuple; Form_pg_propgraph_label pglform; - StringInfoData objdesc; - - initStringInfo(&objdesc); + StringInfoData rel; tuple = SearchSysCache1(PROPGRAPHLABELOID, ObjectIdGetDatum(object->objectId)); if (!HeapTupleIsValid(tuple)) @@ -4158,11 +4153,12 @@ getObjectDescription(const ObjectAddress *object, bool missing_ok) pglform = (Form_pg_propgraph_label) GETSTRUCT(tuple); - getRelationDescription(&objdesc, pglform->pglpgid, false); + initStringInfo(&rel); + getRelationDescription(&rel, pglform->pglpgid, false); - appendStringInfo(&buffer, _("label %s of %s"), NameStr(pglform->pgllabel), objdesc.data); + appendStringInfo(&buffer, _("label %s of %s"), NameStr(pglform->pgllabel), rel.data); - pfree(objdesc.data); + pfree(rel.data); ReleaseSysCache(tuple); break; } @@ -4173,7 +4169,6 @@ getObjectDescription(const ObjectAddress *object, bool missing_ok) HeapTuple tuple; Form_pg_propgraph_label_property plpform; ObjectAddress oa; - char *objdesc; rel = table_open(PropgraphLabelPropertyRelationId, AccessShareLock); tuple = get_catalog_object_by_oid(rel, @@ -4192,11 +4187,11 @@ getObjectDescription(const ObjectAddress *object, bool missing_ok) ObjectAddressSet(oa, PropgraphElementLabelRelationId, plpform->plpellabelid); - objdesc = getObjectDescription(&oa, false); - appendStringInfo(&buffer, _("property %s of %s"), get_propgraph_property_name(plpform->plppropid), objdesc); + appendStringInfo(&buffer, _("property %s of %s"), + get_propgraph_property_name(plpform->plppropid), + getObjectDescription(&oa, false)); - pfree(objdesc); table_close(rel, AccessShareLock); break; } @@ -4205,9 +4200,7 @@ getObjectDescription(const ObjectAddress *object, bool missing_ok) { HeapTuple tuple; Form_pg_propgraph_property pgpform; - StringInfoData objdesc; - - initStringInfo(&objdesc); + StringInfoData rel; tuple = SearchSysCache1(PROPGRAPHPROPOID, ObjectIdGetDatum(object->objectId)); if (!HeapTupleIsValid(tuple)) @@ -4219,10 +4212,11 @@ getObjectDescription(const ObjectAddress *object, bool missing_ok) pgpform = (Form_pg_propgraph_property) GETSTRUCT(tuple); - getRelationDescription(&objdesc, pgpform->pgppgid, false); + initStringInfo(&rel); + getRelationDescription(&rel, pgpform->pgppgid, false); - appendStringInfo(&buffer, _("property %s of %s"), NameStr(pgpform->pgpname), objdesc.data); - pfree(objdesc.data); + appendStringInfo(&buffer, _("property %s of %s"), NameStr(pgpform->pgpname), rel.data); + pfree(rel.data); ReleaseSysCache(tuple); break; } -- 2.34.1
From bd6802810ef76c45814f20d611c3d6a69dd754c5 Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi <[email protected]> Date: Tue, 2 Jun 2026 12:07:58 +0900 Subject: [PATCH v20260602 8/9] Make propgraph object descriptions translatable getObjectDescription() currently constructs propgraph-related object descriptions incrementally with appendStringInfo(). This effectively fixes the word order in English, which makes the messages difficult to translate naturally into languages such as Japanese. --- src/backend/catalog/objectaddress.c | 56 +++++++++++++++++++---------- 1 file changed, 38 insertions(+), 18 deletions(-) diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c index 70058522b35..7b0d163cec9 100644 --- a/src/backend/catalog/objectaddress.c +++ b/src/backend/catalog/objectaddress.c @@ -4077,6 +4077,9 @@ getObjectDescription(const ObjectAddress *object, bool missing_ok) { HeapTuple tup; Form_pg_propgraph_element pgeform; + StringInfoData objdesc; + + initStringInfo(&objdesc); tup = SearchSysCache1(PROPGRAPHELOID, ObjectIdGetDatum(object->objectId)); if (!HeapTupleIsValid(tup)) @@ -4089,16 +4092,16 @@ getObjectDescription(const ObjectAddress *object, bool missing_ok) pgeform = (Form_pg_propgraph_element) GETSTRUCT(tup); + getRelationDescription(&objdesc, pgeform->pgepgid, false); + if (pgeform->pgekind == PGEKIND_VERTEX) - /* translator: followed by, e.g., "property graph %s" */ - appendStringInfo(&buffer, _("vertex %s of "), NameStr(pgeform->pgealias)); + appendStringInfo(&buffer, _("vertex %s of %s"), NameStr(pgeform->pgealias), objdesc.data); else if (pgeform->pgekind == PGEKIND_EDGE) - /* translator: followed by, e.g., "property graph %s" */ - appendStringInfo(&buffer, _("edge %s of "), NameStr(pgeform->pgealias)); + appendStringInfo(&buffer, _("edge %s of %s"), NameStr(pgeform->pgealias), objdesc.data); else - appendStringInfo(&buffer, "??? element %s of ", NameStr(pgeform->pgealias)); - getRelationDescription(&buffer, pgeform->pgepgid, false); + appendStringInfo(&buffer, "??? element %s of %s", NameStr(pgeform->pgealias), objdesc.data); + pfree(objdesc.data); ReleaseSysCache(tup); break; } @@ -4109,6 +4112,7 @@ getObjectDescription(const ObjectAddress *object, bool missing_ok) HeapTuple tuple; Form_pg_propgraph_element_label pgelform; ObjectAddress oa; + char *objdesc; rel = table_open(PropgraphElementLabelRelationId, AccessShareLock); tuple = get_catalog_object_by_oid(rel, @@ -4125,10 +4129,13 @@ getObjectDescription(const ObjectAddress *object, bool missing_ok) pgelform = (Form_pg_propgraph_element_label) GETSTRUCT(tuple); - appendStringInfo(&buffer, _("label %s of "), get_propgraph_label_name(pgelform->pgellabelid)); - ObjectAddressSet(oa, PropgraphElementRelationId, pgelform->pgelelid); - appendStringInfoString(&buffer, getObjectDescription(&oa, false)); + ObjectAddressSet(oa, PropgraphElementRelationId, + pgelform->pgelelid); + objdesc = getObjectDescription(&oa, false); + appendStringInfo(&buffer, _("label %s of %s"), get_propgraph_label_name(pgelform->pgellabelid), objdesc); + + pfree(objdesc); table_close(rel, AccessShareLock); break; } @@ -4137,6 +4144,9 @@ getObjectDescription(const ObjectAddress *object, bool missing_ok) { HeapTuple tuple; Form_pg_propgraph_label pglform; + StringInfoData objdesc; + + initStringInfo(&objdesc); tuple = SearchSysCache1(PROPGRAPHLABELOID, ObjectIdGetDatum(object->objectId)); if (!HeapTupleIsValid(tuple)) @@ -4148,9 +4158,11 @@ getObjectDescription(const ObjectAddress *object, bool missing_ok) pglform = (Form_pg_propgraph_label) GETSTRUCT(tuple); - /* translator: followed by, e.g., "property graph %s" */ - appendStringInfo(&buffer, _("label %s of "), NameStr(pglform->pgllabel)); - getRelationDescription(&buffer, pglform->pglpgid, false); + getRelationDescription(&objdesc, pglform->pglpgid, false); + + appendStringInfo(&buffer, _("label %s of %s"), NameStr(pglform->pgllabel), objdesc.data); + + pfree(objdesc.data); ReleaseSysCache(tuple); break; } @@ -4161,6 +4173,7 @@ getObjectDescription(const ObjectAddress *object, bool missing_ok) HeapTuple tuple; Form_pg_propgraph_label_property plpform; ObjectAddress oa; + char *objdesc; rel = table_open(PropgraphLabelPropertyRelationId, AccessShareLock); tuple = get_catalog_object_by_oid(rel, @@ -4177,10 +4190,13 @@ getObjectDescription(const ObjectAddress *object, bool missing_ok) plpform = (Form_pg_propgraph_label_property) GETSTRUCT(tuple); - appendStringInfo(&buffer, _("property %s of "), get_propgraph_property_name(plpform->plppropid)); - ObjectAddressSet(oa, PropgraphElementLabelRelationId, plpform->plpellabelid); - appendStringInfoString(&buffer, getObjectDescription(&oa, false)); + ObjectAddressSet(oa, PropgraphElementLabelRelationId, + plpform->plpellabelid); + objdesc = getObjectDescription(&oa, false); + + appendStringInfo(&buffer, _("property %s of %s"), get_propgraph_property_name(plpform->plppropid), objdesc); + pfree(objdesc); table_close(rel, AccessShareLock); break; } @@ -4189,6 +4205,9 @@ getObjectDescription(const ObjectAddress *object, bool missing_ok) { HeapTuple tuple; Form_pg_propgraph_property pgpform; + StringInfoData objdesc; + + initStringInfo(&objdesc); tuple = SearchSysCache1(PROPGRAPHPROPOID, ObjectIdGetDatum(object->objectId)); if (!HeapTupleIsValid(tuple)) @@ -4200,9 +4219,10 @@ getObjectDescription(const ObjectAddress *object, bool missing_ok) pgpform = (Form_pg_propgraph_property) GETSTRUCT(tuple); - /* translator: followed by, e.g., "property graph %s" */ - appendStringInfo(&buffer, _("property %s of "), NameStr(pgpform->pgpname)); - getRelationDescription(&buffer, pgpform->pgppgid, false); + getRelationDescription(&objdesc, pgpform->pgppgid, false); + + appendStringInfo(&buffer, _("property %s of %s"), NameStr(pgpform->pgpname), objdesc.data); + pfree(objdesc.data); ReleaseSysCache(tuple); break; } -- 2.34.1
