From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
To: ashutosh.bapat.oss@gmail.com
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: A few message wording/formatting cleanup patches
Date: Tue, 02 Jun 2026 13:43:56 +0900 (JST)
Message-ID: <20260602.134356.653346963021072409.horikyota.ntt@gmail.com> (raw)
In-Reply-To: <CAExHW5uN6qu-=iVtoAyFKDaa2hN0i-NHEjCfGzb=ePxjZcm22Q@mail.gmail.com>
References: <20260528.121622.1662808269492494574.horikyota.ntt@gmail.com>
<CAExHW5uN6qu-=iVtoAyFKDaa2hN0i-NHEjCfGzb=ePxjZcm22Q@mail.gmail.com>
Hello. Thank you for the comments.
At Mon, 1 Jun 2026 17:18:13 +0530, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> 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!
--
Kyotaro Horiguchi
NTT Open Source Software Center
Attachments:v2-0001-Make-propgraph-object-descriptions-translatable.patch (text/x-patch, 6.2K) download
From e8f0a3f18fa052fd0ff2d7fc51ac23401021d8f1 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Date: Tue, 2 Jun 2026 12:07:58 +0900
Subject: [PATCH v2] 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.cindex 050b7829eb0..9eb574f5ccc 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.52.0
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: horikyota.ntt@gmail.com, ashutosh.bapat.oss@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: A few message wording/formatting cleanup patches
In-Reply-To: <20260602.134356.653346963021072409.horikyota.ntt@gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox