agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: 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:
[text/x-patch] v2-0001-Make-propgraph-object-descriptions-translatable.patch (6.2K, ../20260602.134356.653346963021072409.horikyota.ntt@gmail.com/2-v2-0001-Make-propgraph-object-descriptions-translatable.patch)
download | inline diff:
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
view thread (10+ messages) latest in thread
Message-ID: <20260602.134356.653346963021072409.horikyota.ntt@gmail.com>
Permalink: ../20260602.134356.653346963021072409.horikyota.ntt@gmail.com/
Also on: postgresql.org/message-id/20260602.134356.653346963021072409.horikyota.ntt@gmail.com
reply
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 agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox