Date: Tue, 02 Jun 2026 13:43:56 +0900 (JST)
Message-Id: <20260602.134356.653346963021072409.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
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
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>
User-Agent: Mew version 6.8 on Emacs 29.4
Mime-Version: 1.0
Content-Type: Multipart/Mixed;
 boundary="--Next_Part(Tue_Jun__2_13_43_56_2026_572)--"
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/20260602.134356.653346963021072409.horikyota.ntt%40gmail.com>
Precedence: bulk

----Next_Part(Tue_Jun__2_13_43_56_2026_572)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

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

----Next_Part(Tue_Jun__2_13_43_56_2026_572)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="v2-0001-Make-propgraph-object-descriptions-translatable.patch"

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.c
index 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


----Next_Part(Tue_Jun__2_13_43_56_2026_572)----