agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedA few message wording/formatting cleanup patches
10+ messages / 6 participants
[nested] [flat]
* A few message wording/formatting cleanup patches
@ 2026-05-28 03:16 Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-05-28 17:19 ` Re: A few message wording/formatting cleanup patches Daniel Gustafsson <daniel@yesql.se>
2026-06-01 11:48 ` Re: A few message wording/formatting cleanup patches Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
0 siblings, 2 replies; 10+ messages in thread
From: Kyotaro Horiguchi @ 2026-05-28 03:16 UTC (permalink / raw)
To: pgsql-hackers@lists.postgresql.org
Hello,
While translating error messages into Japanese, I came across some
unusually formatted messages and messages that are difficult to
translate naturally into Japanese.
0001: 0001-Make-propgraph-object-descriptions-translatable.patch
getObjectDescription() currently builds the object name for
PropgraphElementRelationId incrementally with appendStringInfo(), but
that works poorly with Japanese because it effectively fixes the word
order. We probably need to construct the whole name from a single
format string instead.
0002: 0002-Use-double-quotes-in-message.patch
fe-protocol3.c has the following message containing backquotes:
> libpq_append_conn_error(conn,
> "server did not report the unsupported `_pq_.test_protocol_negotiation` parameter in its protocol negotiation message");
but that doesn't seem to match our usual style. Nearby messages use
double quotes instead.
0003: 0003-Add-missing-period-to-HINT-message.patch
In be-secure-openssl.c, the following HINT message is missing a
trailing period:
> errhint("If ssl_sni is enabled then add configuration to "%s", else "%s"",
0004: 0004-Use-singular-datachecksum-consistently-in-process-na.patch
In datachecksum_state.c, the launcher process is referred to in two
different ways: "datachecksum launcher" and "datachecksums
launcher". Considering the worker process name, I think the former is
probably the intended one, so this patch makes the naming consistent
accordingly.
That said, I can also imagine an interpretation where "datachecksums"
was chosen intentionally to refer to the checksum feature or checksum
set as a whole, so I'm not entirely sure whether this should be
considered a real issue or just a stylistic inconsistency.
Still, having both forms coexist seems somewhat error-prone in
practice, especially when typing or searching symbol names.
Regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
Attachments:
[text/x-patch] 0001-Make-propgraph-object-descriptions-translatable.patch (6.4K, ../../20260528.121622.1662808269492494574.horikyota.ntt@gmail.com/2-0001-Make-propgraph-object-descriptions-translatable.patch)
download | inline diff:
From 6b8145265ebbb097afc231f0190a8ed8d2dda26d Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Date: Tue, 26 May 2026 16:14:54 +0900
Subject: [PATCH 1/4] 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.
Build the whole object description from a single format string
instead.
---
src/backend/catalog/objectaddress.c | 61 ++++++++++++++++++++---------
1 file changed, 43 insertions(+), 18 deletions(-)
diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c
index 050b7829eb0..c107d0fb0e4 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,15 +4092,15 @@ getObjectDescription(const ObjectAddress *object, bool missing_ok)
pgeform = (Form_pg_propgraph_element) GETSTRUCT(tup);
+ getRelationDescription(&objdesc, pgeform->pgepgid, false);
+ Assert(objdesc.len > 0);
+
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);
ReleaseSysCache(tup);
break;
@@ -4109,6 +4112,9 @@ getObjectDescription(const ObjectAddress *object, bool missing_ok)
HeapTuple tuple;
Form_pg_propgraph_element_label pgelform;
ObjectAddress oa;
+ StringInfoData objdesc;
+
+ initStringInfo(&objdesc);
rel = table_open(PropgraphElementLabelRelationId, AccessShareLock);
tuple = get_catalog_object_by_oid(rel,
@@ -4125,9 +4131,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);
+ appendStringInfoString(&objdesc,
+ getObjectDescription(&oa, false));
+ Assert(objdesc.len > 0);
+
+ appendStringInfo(&buffer, _("label %s of %s"), get_propgraph_label_name(pgelform->pgellabelid), objdesc.data);
table_close(rel, AccessShareLock);
break;
@@ -4137,6 +4147,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 +4161,10 @@ 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);
+ Assert(objdesc.len > 0);
+
+ appendStringInfo(&buffer, _("label %s of %s"), NameStr(pglform->pgllabel), objdesc.data);
ReleaseSysCache(tuple);
break;
}
@@ -4161,6 +4175,9 @@ getObjectDescription(const ObjectAddress *object, bool missing_ok)
HeapTuple tuple;
Form_pg_propgraph_label_property plpform;
ObjectAddress oa;
+ StringInfoData objdesc;
+
+ initStringInfo(&objdesc);
rel = table_open(PropgraphLabelPropertyRelationId, AccessShareLock);
tuple = get_catalog_object_by_oid(rel,
@@ -4177,9 +4194,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);
+ appendStringInfoString(&objdesc,
+ getObjectDescription(&oa, false));
+ Assert(objdesc.len > 0);
+
+ appendStringInfo(&buffer, _("property %s of %s"), get_propgraph_property_name(plpform->plppropid), objdesc.data);
table_close(rel, AccessShareLock);
break;
@@ -4189,6 +4210,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 +4224,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);
+ Assert(objdesc.len > 0);
+
+ appendStringInfo(&buffer, _("property %s of %s"), NameStr(pgpform->pgpname), objdesc.data);
ReleaseSysCache(tuple);
break;
}
--
2.47.3
[text/x-patch] 0002-Use-double-quotes-in-message.patch (1.1K, ../../20260528.121622.1662808269492494574.horikyota.ntt@gmail.com/3-0002-Use-double-quotes-in-message.patch)
download | inline diff:
From 5e073e8017c241638a28e20d9df1ff5d7fb994aa Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Date: Tue, 26 May 2026 16:17:56 +0900
Subject: [PATCH 2/4] Use double quotes in message
Replace backquotes around a symbol name in a user-facing message with
double quotes for consistency with other messages.
---
src/interfaces/libpq/fe-protocol3.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c
index 840e018cd18..4efa8a16ab1 100644
--- a/src/interfaces/libpq/fe-protocol3.c
+++ b/src/interfaces/libpq/fe-protocol3.c
@@ -1560,7 +1560,7 @@ pqGetNegotiateProtocolVersion3(PGconn *conn)
*/
if (expect_test_protocol_negotiation && !found_test_protocol_negotiation)
{
- libpq_append_conn_error(conn, "server did not report the unsupported `_pq_.test_protocol_negotiation` parameter in its protocol negotiation message");
+ libpq_append_conn_error(conn, "server did not report the unsupported \"_pq_.test_protocol_negotiation\" parameter in its protocol negotiation message");
goto failure;
}
--
2.47.3
[text/x-patch] 0003-Add-missing-period-to-HINT-message.patch (1004B, ../../20260528.121622.1662808269492494574.horikyota.ntt@gmail.com/4-0003-Add-missing-period-to-HINT-message.patch)
download | inline diff:
From 5169b2082ae478d8fd0710cd725952be2cc9deb5 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Date: Thu, 28 May 2026 11:53:29 +0900
Subject: [PATCH 3/4] Add missing period to HINT message
Add a trailing period to a HINT message.
---
src/backend/libpq/be-secure-openssl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index f2738c351f9..e81d2eab3fd 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -364,7 +364,7 @@ be_tls_init(bool isServerStart)
errcode(ERRCODE_CONFIG_FILE_ERROR),
errmsg("no SSL configurations loaded"),
/*- translator: The two %s contain filenames */
- errhint("If ssl_sni is enabled then add configuration to \"%s\", else \"%s\"",
+ errhint("If ssl_sni is enabled then add configuration to \"%s\", else \"%s\".",
"pg_hosts.conf", "postgresql.conf"));
goto error;
}
--
2.47.3
[text/x-patch] 0004-Use-singular-datachecksum-consistently-in-process-na.patch (37.7K, ../../20260528.121622.1662808269492494574.horikyota.ntt@gmail.com/5-0004-Use-singular-datachecksum-consistently-in-process-na.patch)
download | inline diff:
From 708860830b9537da5d37e9f8a7dbf10a070d7e78 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Date: Tue, 26 May 2026 16:56:01 +0900
Subject: [PATCH 4/4] Use singular "datachecksum" consistently in process names
datachecksum_state.c currently refers to processes as both
"datachecksum worker/launcher" and "datachecksums launcher".
Use the singular "datachecksum" form consistently for process names
instead.
---
src/backend/access/transam/xlog.c | 4 +-
src/backend/postmaster/bgworker.c | 8 +-
src/backend/postmaster/datachecksum_state.c | 234 ++++++++++----------
src/backend/postmaster/postmaster.c | 4 +-
src/backend/storage/ipc/procsignal.c | 2 +-
src/backend/utils/activity/pgstat_backend.c | 4 +-
src/backend/utils/activity/pgstat_io.c | 4 +-
src/backend/utils/init/miscinit.c | 2 +-
src/backend/utils/init/postinit.c | 2 +-
src/include/miscadmin.h | 10 +-
src/include/postmaster/datachecksum_state.h | 30 +--
src/include/postmaster/proctypelist.h | 4 +-
src/include/storage/subsystemlist.h | 2 +-
13 files changed, 155 insertions(+), 155 deletions(-)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index fecdf0d4b05..16f8278809e 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -9178,7 +9178,7 @@ xlog_redo(XLogReaderState *record)
SpinLockRelease(&XLogCtl->info_lck);
if (new_state)
- EmitAndWaitDataChecksumsBarrier(redo_rec.data_checksum_version);
+ EmitAndWaitDataChecksumBarrier(redo_rec.data_checksum_version);
}
else if (info == XLOG_LOGICAL_DECODING_STATUS_CHANGE)
{
@@ -9256,7 +9256,7 @@ xlog2_redo(XLogReaderState *record)
* change to checksum status. Once the barrier has been passed we can
* initiate the corresponding processing.
*/
- EmitAndWaitDataChecksumsBarrier(state.new_checksum_state);
+ EmitAndWaitDataChecksumBarrier(state.new_checksum_state);
}
}
diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c
index 2e4acad4f00..68256d90cef 100644
--- a/src/backend/postmaster/bgworker.c
+++ b/src/backend/postmaster/bgworker.c
@@ -160,12 +160,12 @@ static const struct
.fn_addr = TableSyncWorkerMain
},
{
- .fn_name = "DataChecksumsWorkerLauncherMain",
- .fn_addr = DataChecksumsWorkerLauncherMain
+ .fn_name = "DataChecksumWorkerLauncherMain",
+ .fn_addr = DataChecksumWorkerLauncherMain
},
{
- .fn_name = "DataChecksumsWorkerMain",
- .fn_addr = DataChecksumsWorkerMain
+ .fn_name = "DataChecksumWorkerMain",
+ .fn_addr = DataChecksumWorkerMain
}
};
diff --git a/src/backend/postmaster/datachecksum_state.c b/src/backend/postmaster/datachecksum_state.c
index 33430147ff2..1715e506cbc 100644
--- a/src/backend/postmaster/datachecksum_state.c
+++ b/src/backend/postmaster/datachecksum_state.c
@@ -26,7 +26,7 @@
* checksums enabled, then disabled them and updated the page while they were
* disabled.
*
- * The DataChecksumsWorker will compile a list of all databases at the start,
+ * The DataChecksumWorker will compile a list of all databases at the start,
* any databases created concurrently will see the in-progress state and will
* be checksummed automatically. All databases from the original list MUST BE
* successfully processed in order for data checksums to be enabled, the only
@@ -99,10 +99,10 @@
* state will also be set to "off".
*
* Backends transition Bd -> Bi via a procsignalbarrier which is emitted by the
- * DataChecksumsWorkerLauncherMain. When all backends have acknowledged the
+ * DataChecksumWorkerLauncherMain. When all backends have acknowledged the
* barrier then Bd will be empty and the next phase can begin: calculating and
- * writing data checksums with DataChecksumsWorkers. When the
- * DataChecksumsWorker processes have finished writing checksums on all pages,
+ * writing data checksums with DataChecksumWorkers. When the
+ * DataChecksumWorker processes have finished writing checksums on all pages,
* data checksums are enabled cluster-wide via another procsignalbarrier.
* There are four sets of backends where Bd shall be an empty set:
*
@@ -155,7 +155,7 @@
* found on the -hackers threads linked to in the commit message of this
* feature.
*
- * * Launching datachecksumsworker for resuming operation from the startup
+ * * Launching datachecksumworker for resuming operation from the startup
* process: Currently users have to restart processing manually after a
* restart since dynamic background worker cannot be started from the
* postmaster. Changing the startup process could make restarting the
@@ -280,15 +280,15 @@ static const ChecksumBarrierCondition checksum_barriers[9] =
* Signaling between backends calling pg_enable/disable_data_checksums, the
* checksums launcher process, and the checksums worker process.
*
- * This struct is protected by DataChecksumsWorkerLock
+ * This struct is protected by DataChecksumWorkerLock
*/
-typedef struct DataChecksumsStateStruct
+typedef struct DataChecksumStateStruct
{
/*
* These are set by pg_{enable|disable}_data_checksums, to tell the
* launcher what the target state is.
*/
- DataChecksumsWorkerOperation launch_operation;
+ DataChecksumWorkerOperation launch_operation;
int launch_cost_delay;
int launch_cost_limit;
@@ -315,7 +315,7 @@ typedef struct DataChecksumsStateStruct
* without a lock. If multiple workers, or dynamic cost parameters, are
* supported at some point then this would need to be revisited.
*/
- DataChecksumsWorkerOperation operation;
+ DataChecksumWorkerOperation operation;
int cost_delay;
int cost_limit;
@@ -329,58 +329,58 @@ typedef struct DataChecksumsStateStruct
*/
/* result, set by worker before exiting */
- DataChecksumsWorkerResult success;
+ DataChecksumWorkerResult success;
/*
* Tells the worker process whether it should also process the shared
* catalogs
*/
bool process_shared_catalogs;
-} DataChecksumsStateStruct;
+} DataChecksumStateStruct;
-/* Shared memory segment for datachecksumsworker */
-static DataChecksumsStateStruct *DataChecksumState;
+/* Shared memory segment for datachecksumworker */
+static DataChecksumStateStruct *DataChecksumState;
-typedef struct DataChecksumsWorkerDatabase
+typedef struct DataChecksumWorkerDatabase
{
Oid dboid;
char *dbname;
-} DataChecksumsWorkerDatabase;
+} DataChecksumWorkerDatabase;
/* Flag set by the interrupt handler */
static volatile sig_atomic_t abort_requested = false;
/*
- * Have we set the DataChecksumsStateStruct->launcher_running flag?
+ * Have we set the DataChecksumStateStruct->launcher_running flag?
* If we have, we need to clear it before exiting!
*/
static volatile sig_atomic_t launcher_running = false;
/* Are we enabling data checksums, or disabling them? */
-static DataChecksumsWorkerOperation operation;
+static DataChecksumWorkerOperation operation;
/* Prototypes */
-static void DataChecksumsShmemRequest(void *arg);
+static void DataChecksumShmemRequest(void *arg);
static bool DatabaseExists(Oid dboid);
static List *BuildDatabaseList(void);
static List *BuildRelationList(bool temp_relations, bool include_shared);
static void FreeDatabaseList(List *dblist);
-static DataChecksumsWorkerResult ProcessDatabase(DataChecksumsWorkerDatabase *db);
+static DataChecksumWorkerResult ProcessDatabase(DataChecksumWorkerDatabase *db);
static bool ProcessAllDatabases(void);
static bool ProcessSingleRelationFork(Relation reln, ForkNumber forkNum, BufferAccessStrategy strategy);
static void launcher_cancel_handler(SIGNAL_ARGS);
static void WaitForAllTransactionsToFinish(void);
-const ShmemCallbacks DataChecksumsShmemCallbacks = {
- .request_fn = DataChecksumsShmemRequest,
+const ShmemCallbacks DataChecksumShmemCallbacks = {
+ .request_fn = DataChecksumShmemRequest,
};
#define CHECK_FOR_ABORT_REQUEST() \
do { \
- LWLockAcquire(DataChecksumsWorkerLock, LW_SHARED); \
+ LWLockAcquire(DataChecksumWorkerLock, LW_SHARED); \
if (DataChecksumState->launch_operation != operation) \
abort_requested = true; \
- LWLockRelease(DataChecksumsWorkerLock); \
+ LWLockRelease(DataChecksumWorkerLock); \
} while (0)
@@ -389,7 +389,7 @@ const ShmemCallbacks DataChecksumsShmemCallbacks = {
*/
void
-EmitAndWaitDataChecksumsBarrier(uint32 state)
+EmitAndWaitDataChecksumBarrier(uint32 state)
{
uint64 barrier;
@@ -421,7 +421,7 @@ EmitAndWaitDataChecksumsBarrier(uint32 state)
}
/*
- * AbsorbDataChecksumsBarrier
+ * AbsorbDataChecksumBarrier
* Generic function for absorbing data checksum state changes
*
* All procsignalbarriers regarding data checksum state changes are absorbed
@@ -430,7 +430,7 @@ EmitAndWaitDataChecksumsBarrier(uint32 state)
* used to look up the relevant entry.
*/
bool
-AbsorbDataChecksumsBarrier(ProcSignalBarrierType barrier)
+AbsorbDataChecksumBarrier(ProcSignalBarrierType barrier)
{
uint32 target_state;
int current = data_checksums;
@@ -516,7 +516,7 @@ disable_data_checksums(PG_FUNCTION_ARGS)
errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("must be superuser to change data checksum state"));
- StartDataChecksumsWorkerLauncher(DISABLE_DATACHECKSUMS, 0, 0);
+ StartDataChecksumWorkerLauncher(DISABLE_DATACHECKSUMS, 0, 0);
PG_RETURN_VOID();
}
@@ -548,27 +548,27 @@ enable_data_checksums(PG_FUNCTION_ARGS)
errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("cost limit must be greater than zero"));
- StartDataChecksumsWorkerLauncher(ENABLE_DATACHECKSUMS, cost_delay, cost_limit);
+ StartDataChecksumWorkerLauncher(ENABLE_DATACHECKSUMS, cost_delay, cost_limit);
PG_RETURN_VOID();
}
/*****************************************************************************
- * Functionality for running the datachecksumsworker and associated launcher
+ * Functionality for running the datachecksum worker and associated launcher
*/
/*
- * StartDataChecksumsWorkerLauncher
- * Main entry point for datachecksumsworker launcher process
+ * StartDataChecksumWorkerLauncher
+ * Main entry point for datachecksumworker launcher process
*
* The main entrypoint for starting data checksums processing for enabling as
* well as disabling.
*/
void
-StartDataChecksumsWorkerLauncher(DataChecksumsWorkerOperation op,
- int cost_delay,
- int cost_limit)
+StartDataChecksumWorkerLauncher(DataChecksumWorkerOperation op,
+ int cost_delay,
+ int cost_limit)
{
BackgroundWorker bgw;
BackgroundWorkerHandle *bgw_handle;
@@ -580,10 +580,10 @@ StartDataChecksumsWorkerLauncher(DataChecksumsWorkerOperation op,
Assert(cost_delay == 0 && cost_limit == 0);
#endif
- INJECTION_POINT("datachecksumsworker-startup-delay", NULL);
+ INJECTION_POINT("datachecksumworker-startup-delay", NULL);
/* Store the desired state in shared memory */
- LWLockAcquire(DataChecksumsWorkerLock, LW_EXCLUSIVE);
+ LWLockAcquire(DataChecksumWorkerLock, LW_EXCLUSIVE);
DataChecksumState->launch_operation = op;
DataChecksumState->launch_cost_delay = cost_delay;
@@ -592,7 +592,7 @@ StartDataChecksumsWorkerLauncher(DataChecksumsWorkerOperation op,
/* Is the launcher already running? If so, what is it doing? */
running = DataChecksumState->launcher_running;
- LWLockRelease(DataChecksumsWorkerLock);
+ LWLockRelease(DataChecksumWorkerLock);
/*
* Launch a new launcher process, if it's not running already.
@@ -625,7 +625,7 @@ StartDataChecksumsWorkerLauncher(DataChecksumsWorkerOperation op,
bgw.bgw_flags = BGWORKER_SHMEM_ACCESS | BGWORKER_BACKEND_DATABASE_CONNECTION;
bgw.bgw_start_time = BgWorkerStart_RecoveryFinished;
snprintf(bgw.bgw_library_name, BGW_MAXLEN, "postgres");
- snprintf(bgw.bgw_function_name, BGW_MAXLEN, "DataChecksumsWorkerLauncherMain");
+ snprintf(bgw.bgw_function_name, BGW_MAXLEN, "DataChecksumWorkerLauncherMain");
snprintf(bgw.bgw_name, BGW_MAXLEN, "datachecksum launcher");
snprintf(bgw.bgw_type, BGW_MAXLEN, "datachecksum launcher");
bgw.bgw_restart_time = BGW_NEVER_RESTART;
@@ -715,10 +715,10 @@ ProcessSingleRelationFork(Relation reln, ForkNumber forkNum, BufferAccessStrateg
* abortion will bubble up from here.
*/
Assert(operation == ENABLE_DATACHECKSUMS);
- LWLockAcquire(DataChecksumsWorkerLock, LW_SHARED);
+ LWLockAcquire(DataChecksumWorkerLock, LW_SHARED);
if (DataChecksumState->launch_operation == DISABLE_DATACHECKSUMS)
abort_requested = true;
- LWLockRelease(DataChecksumsWorkerLock);
+ LWLockRelease(DataChecksumWorkerLock);
if (abort_requested)
return false;
@@ -795,8 +795,8 @@ ProcessSingleRelationByOid(Oid relationId, BufferAccessStrategy strategy)
* waiting for it to finish. We have to do this in a separate worker, since
* each process can only be connected to one database during its lifetime.
*/
-static DataChecksumsWorkerResult
-ProcessDatabase(DataChecksumsWorkerDatabase *db)
+static DataChecksumWorkerResult
+ProcessDatabase(DataChecksumWorkerDatabase *db)
{
BackgroundWorker bgw;
BackgroundWorkerHandle *bgw_handle;
@@ -804,15 +804,15 @@ ProcessDatabase(DataChecksumsWorkerDatabase *db)
pid_t pid;
char activity[NAMEDATALEN + 64];
- LWLockAcquire(DataChecksumsWorkerLock, LW_EXCLUSIVE);
- DataChecksumState->success = DATACHECKSUMSWORKER_FAILED;
- LWLockRelease(DataChecksumsWorkerLock);
+ LWLockAcquire(DataChecksumWorkerLock, LW_EXCLUSIVE);
+ DataChecksumState->success = DATACHECKSUMWORKER_FAILED;
+ LWLockRelease(DataChecksumWorkerLock);
memset(&bgw, 0, sizeof(bgw));
bgw.bgw_flags = BGWORKER_SHMEM_ACCESS | BGWORKER_BACKEND_DATABASE_CONNECTION;
bgw.bgw_start_time = BgWorkerStart_RecoveryFinished;
snprintf(bgw.bgw_library_name, BGW_MAXLEN, "postgres");
- snprintf(bgw.bgw_function_name, BGW_MAXLEN, "%s", "DataChecksumsWorkerMain");
+ snprintf(bgw.bgw_function_name, BGW_MAXLEN, "%s", "DataChecksumWorkerMain");
snprintf(bgw.bgw_name, BGW_MAXLEN, "datachecksum worker");
snprintf(bgw.bgw_type, BGW_MAXLEN, "datachecksum worker");
bgw.bgw_restart_time = BGW_NEVER_RESTART;
@@ -830,7 +830,7 @@ ProcessDatabase(DataChecksumsWorkerDatabase *db)
errmsg("could not start background worker for enabling data checksums in database \"%s\"",
db->dbname),
errhint("The \"%s\" setting might be too low.", "max_worker_processes"));
- return DATACHECKSUMSWORKER_FAILED;
+ return DATACHECKSUMWORKER_FAILED;
}
status = WaitForBackgroundWorkerStartup(bgw_handle, &pid);
@@ -840,17 +840,17 @@ ProcessDatabase(DataChecksumsWorkerDatabase *db)
* If the worker managed to start, and stop, before we got to waiting
* for it we can see a STOPPED status here without it being a failure.
*/
- LWLockAcquire(DataChecksumsWorkerLock, LW_SHARED);
- if (DataChecksumState->success == DATACHECKSUMSWORKER_SUCCESSFUL)
+ LWLockAcquire(DataChecksumWorkerLock, LW_SHARED);
+ if (DataChecksumState->success == DATACHECKSUMWORKER_SUCCESSFUL)
{
- LWLockRelease(DataChecksumsWorkerLock);
+ LWLockRelease(DataChecksumWorkerLock);
pgstat_report_activity(STATE_IDLE, NULL);
- LWLockAcquire(DataChecksumsWorkerLock, LW_EXCLUSIVE);
+ LWLockAcquire(DataChecksumWorkerLock, LW_EXCLUSIVE);
DataChecksumState->worker_pid = InvalidPid;
- LWLockRelease(DataChecksumsWorkerLock);
+ LWLockRelease(DataChecksumWorkerLock);
return DataChecksumState->success;
}
- LWLockRelease(DataChecksumsWorkerLock);
+ LWLockRelease(DataChecksumWorkerLock);
ereport(WARNING,
errmsg("could not start background worker for enabling data checksums in database \"%s\"",
@@ -862,9 +862,9 @@ ProcessDatabase(DataChecksumsWorkerDatabase *db)
* treat it as not an error, else treat as fatal and error out.
*/
if (DatabaseExists(db->dboid))
- return DATACHECKSUMSWORKER_FAILED;
+ return DATACHECKSUMWORKER_FAILED;
else
- return DATACHECKSUMSWORKER_DROPDB;
+ return DATACHECKSUMWORKER_DROPDB;
}
/*
@@ -886,9 +886,9 @@ ProcessDatabase(DataChecksumsWorkerDatabase *db)
db->dbname));
/* Save the pid of the worker so we can signal it later */
- LWLockAcquire(DataChecksumsWorkerLock, LW_EXCLUSIVE);
+ LWLockAcquire(DataChecksumWorkerLock, LW_EXCLUSIVE);
DataChecksumState->worker_pid = pid;
- LWLockRelease(DataChecksumsWorkerLock);
+ LWLockRelease(DataChecksumWorkerLock);
snprintf(activity, sizeof(activity) - 1,
"Waiting for worker in database %s (pid %ld)", db->dbname, (long) pid);
@@ -902,17 +902,17 @@ ProcessDatabase(DataChecksumsWorkerDatabase *db)
db->dbname),
errhint("Restart the database and restart data checksum processing by calling pg_enable_data_checksums()."));
- LWLockAcquire(DataChecksumsWorkerLock, LW_SHARED);
- if (DataChecksumState->success == DATACHECKSUMSWORKER_ABORTED)
+ LWLockAcquire(DataChecksumWorkerLock, LW_SHARED);
+ if (DataChecksumState->success == DATACHECKSUMWORKER_ABORTED)
ereport(LOG,
errmsg("data checksums processing was aborted in database \"%s\"",
db->dbname));
- LWLockRelease(DataChecksumsWorkerLock);
+ LWLockRelease(DataChecksumWorkerLock);
pgstat_report_activity(STATE_IDLE, NULL);
- LWLockAcquire(DataChecksumsWorkerLock, LW_EXCLUSIVE);
+ LWLockAcquire(DataChecksumWorkerLock, LW_EXCLUSIVE);
DataChecksumState->worker_pid = InvalidPid;
- LWLockRelease(DataChecksumsWorkerLock);
+ LWLockRelease(DataChecksumWorkerLock);
return DataChecksumState->success;
}
@@ -934,14 +934,14 @@ launcher_exit(int code, Datum arg)
if (launcher_running)
{
- LWLockAcquire(DataChecksumsWorkerLock, LW_EXCLUSIVE);
+ LWLockAcquire(DataChecksumWorkerLock, LW_EXCLUSIVE);
if (DataChecksumState->worker_pid != InvalidPid)
{
ereport(LOG,
errmsg("data checksums launcher exiting while worker is still running, signalling worker"));
kill(DataChecksumState->worker_pid, SIGTERM);
}
- LWLockRelease(DataChecksumsWorkerLock);
+ LWLockRelease(DataChecksumWorkerLock);
}
/*
@@ -951,10 +951,10 @@ launcher_exit(int code, Datum arg)
if (DataChecksumsInProgressOn())
SetDataChecksumsOff();
- LWLockAcquire(DataChecksumsWorkerLock, LW_EXCLUSIVE);
+ LWLockAcquire(DataChecksumWorkerLock, LW_EXCLUSIVE);
launcher_running = false;
DataChecksumState->launcher_running = false;
- LWLockRelease(DataChecksumsWorkerLock);
+ LWLockRelease(DataChecksumWorkerLock);
}
/*
@@ -1044,7 +1044,7 @@ WaitForAllTransactionsToFinish(void)
}
/*
- * DataChecksumsWorkerLauncherMain
+ * DataChecksumWorkerLauncherMain
*
* Main function for launching dynamic background workers for processing data
* checksums in databases. This function has the bgworker management, with
@@ -1052,11 +1052,11 @@ WaitForAllTransactionsToFinish(void)
* initiating processing.
*/
void
-DataChecksumsWorkerLauncherMain(Datum arg)
+DataChecksumWorkerLauncherMain(Datum arg)
{
ereport(DEBUG1,
- errmsg("background worker \"datachecksums launcher\" started"));
+ errmsg("background worker \"datachecksum launcher\" started"));
pqsignal(SIGTERM, die);
pqsignal(SIGINT, launcher_cancel_handler);
@@ -1065,19 +1065,19 @@ DataChecksumsWorkerLauncherMain(Datum arg)
BackgroundWorkerUnblockSignals();
- MyBackendType = B_DATACHECKSUMSWORKER_LAUNCHER;
+ MyBackendType = B_DATACHECKSUMWORKER_LAUNCHER;
init_ps_display(NULL);
- INJECTION_POINT("datachecksumsworker-launcher-delay", NULL);
+ INJECTION_POINT("datachecksumworker-launcher-delay", NULL);
- LWLockAcquire(DataChecksumsWorkerLock, LW_EXCLUSIVE);
+ LWLockAcquire(DataChecksumWorkerLock, LW_EXCLUSIVE);
if (DataChecksumState->launcher_running)
{
ereport(LOG,
- errmsg("background worker \"datachecksums launcher\" already running, exiting"));
+ errmsg("background worker \"datachecksum launcher\" already running, exiting"));
/* Launcher was already running, let it finish */
- LWLockRelease(DataChecksumsWorkerLock);
+ LWLockRelease(DataChecksumWorkerLock);
return;
}
@@ -1092,7 +1092,7 @@ DataChecksumsWorkerLauncherMain(Datum arg)
DataChecksumState->operation = operation;
DataChecksumState->cost_delay = DataChecksumState->launch_cost_delay;
DataChecksumState->cost_limit = DataChecksumState->launch_cost_limit;
- LWLockRelease(DataChecksumsWorkerLock);
+ LWLockRelease(DataChecksumWorkerLock);
/*
* The target state can change while we are busy enabling/disabling
@@ -1134,13 +1134,13 @@ again:
* If the target state changed during processing then it's not a
* failure, so restart processing instead.
*/
- LWLockAcquire(DataChecksumsWorkerLock, LW_EXCLUSIVE);
+ LWLockAcquire(DataChecksumWorkerLock, LW_EXCLUSIVE);
if (DataChecksumState->launch_operation != operation)
{
- LWLockRelease(DataChecksumsWorkerLock);
+ LWLockRelease(DataChecksumWorkerLock);
goto done;
}
- LWLockRelease(DataChecksumsWorkerLock);
+ LWLockRelease(DataChecksumWorkerLock);
ereport(ERROR,
errcode(ERRCODE_INSUFFICIENT_RESOURCES),
errmsg("unable to enable data checksums in cluster"));
@@ -1183,14 +1183,14 @@ done:
* while we were running. In that case we will have to start all over
* again.
*/
- LWLockAcquire(DataChecksumsWorkerLock, LW_EXCLUSIVE);
+ LWLockAcquire(DataChecksumWorkerLock, LW_EXCLUSIVE);
if (DataChecksumState->launch_operation != operation)
{
DataChecksumState->operation = DataChecksumState->launch_operation;
operation = DataChecksumState->launch_operation;
DataChecksumState->cost_delay = DataChecksumState->launch_cost_delay;
DataChecksumState->cost_limit = DataChecksumState->launch_cost_limit;
- LWLockRelease(DataChecksumsWorkerLock);
+ LWLockRelease(DataChecksumWorkerLock);
goto again;
}
@@ -1199,7 +1199,7 @@ done:
launcher_running = false;
DataChecksumState->launcher_running = false;
- LWLockRelease(DataChecksumsWorkerLock);
+ LWLockRelease(DataChecksumWorkerLock);
}
/*
@@ -1217,9 +1217,9 @@ ProcessAllDatabases(void)
int cumulative_total = 0;
/* Set up so first run processes shared catalogs, not once in every db */
- LWLockAcquire(DataChecksumsWorkerLock, LW_EXCLUSIVE);
+ LWLockAcquire(DataChecksumWorkerLock, LW_EXCLUSIVE);
DataChecksumState->process_shared_catalogs = true;
- LWLockRelease(DataChecksumsWorkerLock);
+ LWLockRelease(DataChecksumWorkerLock);
/* Get a list of all databases to process */
WaitForAllTransactionsToFinish();
@@ -1254,18 +1254,18 @@ ProcessAllDatabases(void)
pgstat_progress_update_multi_param(6, index, vals);
}
- foreach_ptr(DataChecksumsWorkerDatabase, db, DatabaseList)
+ foreach_ptr(DataChecksumWorkerDatabase, db, DatabaseList)
{
- DataChecksumsWorkerResult result;
+ DataChecksumWorkerResult result;
result = ProcessDatabase(db);
#ifdef USE_INJECTION_POINTS
/* Allow a test process to alter the result of the operation */
- if (IS_INJECTION_POINT_ATTACHED("datachecksumsworker-fail-db-result"))
+ if (IS_INJECTION_POINT_ATTACHED("datachecksumworker-fail-db-result"))
{
- result = DATACHECKSUMSWORKER_FAILED;
- INJECTION_POINT_CACHED("datachecksumsworker-fail-db-result",
+ result = DATACHECKSUMWORKER_FAILED;
+ INJECTION_POINT_CACHED("datachecksumworker-fail-db-result",
db->dbname);
}
#endif
@@ -1273,7 +1273,7 @@ ProcessAllDatabases(void)
pgstat_progress_update_param(PROGRESS_DATACHECKSUMS_DBS_DONE,
++cumulative_total);
- if (result == DATACHECKSUMSWORKER_FAILED)
+ if (result == DATACHECKSUMWORKER_FAILED)
{
/*
* Disable checksums on cluster, because we failed one of the
@@ -1285,7 +1285,7 @@ ProcessAllDatabases(void)
errmsg("data checksums failed to get enabled in all databases, aborting"),
errhint("The server log might have more information on the cause of the error."));
}
- else if (result == DATACHECKSUMSWORKER_ABORTED || abort_requested)
+ else if (result == DATACHECKSUMWORKER_ABORTED || abort_requested)
{
/* Abort flag set, so exit the whole process */
return false;
@@ -1295,9 +1295,9 @@ ProcessAllDatabases(void)
* When one database has completed, it will have done shared catalogs
* so we don't have to process them again.
*/
- LWLockAcquire(DataChecksumsWorkerLock, LW_EXCLUSIVE);
+ LWLockAcquire(DataChecksumWorkerLock, LW_EXCLUSIVE);
DataChecksumState->process_shared_catalogs = false;
- LWLockRelease(DataChecksumsWorkerLock);
+ LWLockRelease(DataChecksumWorkerLock);
}
FreeDatabaseList(DatabaseList);
@@ -1308,14 +1308,14 @@ ProcessAllDatabases(void)
}
/*
- * DataChecksumsShmemRequest
- * Request datachecksumsworker-related shared memory
+ * DataChecksumShmemRequest
+ * Request datachecksumworker-related shared memory
*/
static void
-DataChecksumsShmemRequest(void *arg)
+DataChecksumShmemRequest(void *arg)
{
- ShmemRequestStruct(.name = "DataChecksumsWorker Data",
- .size = sizeof(DataChecksumsStateStruct),
+ ShmemRequestStruct(.name = "DataChecksumWorker Data",
+ .size = sizeof(DataChecksumStateStruct),
.ptr = (void **) &DataChecksumState,
);
}
@@ -1370,7 +1370,7 @@ DatabaseExists(Oid dboid)
* BuildDatabaseList
* Compile a list of all currently available databases in the cluster
*
- * This creates the list of databases for the datachecksumsworker workers to
+ * This creates the list of databases for the datachecksum workers to
* add checksums to. If the caller wants to ensure that no concurrently
* running CREATE DATABASE calls exist, this needs to be preceded by a call
* to WaitForAllTransactionsToFinish().
@@ -1393,11 +1393,11 @@ BuildDatabaseList(void)
while (HeapTupleIsValid(tup = heap_getnext(scan, ForwardScanDirection)))
{
Form_pg_database pgdb = (Form_pg_database) GETSTRUCT(tup);
- DataChecksumsWorkerDatabase *db;
+ DataChecksumWorkerDatabase *db;
oldctx = MemoryContextSwitchTo(ctx);
- db = (DataChecksumsWorkerDatabase *) palloc0(sizeof(DataChecksumsWorkerDatabase));
+ db = (DataChecksumWorkerDatabase *) palloc0(sizeof(DataChecksumWorkerDatabase));
db->dboid = pgdb->oid;
db->dbname = pstrdup(NameStr(pgdb->datname));
@@ -1421,7 +1421,7 @@ FreeDatabaseList(List *dblist)
if (!dblist)
return;
- foreach_ptr(DataChecksumsWorkerDatabase, db, dblist)
+ foreach_ptr(DataChecksumWorkerDatabase, db, dblist)
{
if (db->dbname != NULL)
pfree(db->dbname);
@@ -1496,7 +1496,7 @@ BuildRelationList(bool temp_relations, bool include_shared)
}
/*
- * DataChecksumsWorkerMain
+ * DataChecksumWorkerMain
*
* Main function for enabling checksums in a single database. This is the
* function set as the bgw_function_name in the dynamic background worker
@@ -1507,7 +1507,7 @@ BuildRelationList(bool temp_relations, bool include_shared)
* existing temporary relations with data checksums.
*/
void
-DataChecksumsWorkerMain(Datum arg)
+DataChecksumWorkerMain(Datum arg)
{
Oid dboid = DatumGetObjectId(arg);
List *RelationList = NIL;
@@ -1526,7 +1526,7 @@ DataChecksumsWorkerMain(Datum arg)
BackgroundWorkerUnblockSignals();
- MyBackendType = B_DATACHECKSUMSWORKER_WORKER;
+ MyBackendType = B_DATACHECKSUMWORKER_WORKER;
init_ps_display(NULL);
BackgroundWorkerInitializeConnectionByOid(dboid, InvalidOid,
@@ -1606,7 +1606,7 @@ DataChecksumsWorkerMain(Datum arg)
* to reflect the new values and signal that the access strategy needs
* to be refreshed.
*/
- LWLockAcquire(DataChecksumsWorkerLock, LW_EXCLUSIVE);
+ LWLockAcquire(DataChecksumWorkerLock, LW_EXCLUSIVE);
if ((DataChecksumState->launch_cost_delay != DataChecksumState->cost_delay)
|| (DataChecksumState->launch_cost_limit != DataChecksumState->cost_limit))
{
@@ -1620,7 +1620,7 @@ DataChecksumsWorkerMain(Datum arg)
}
else
costs_updated = false;
- LWLockRelease(DataChecksumsWorkerLock);
+ LWLockRelease(DataChecksumWorkerLock);
if (costs_updated)
{
@@ -1634,9 +1634,9 @@ DataChecksumsWorkerMain(Datum arg)
if (aborted || abort_requested)
{
- LWLockAcquire(DataChecksumsWorkerLock, LW_EXCLUSIVE);
- DataChecksumState->success = DATACHECKSUMSWORKER_ABORTED;
- LWLockRelease(DataChecksumsWorkerLock);
+ LWLockAcquire(DataChecksumWorkerLock, LW_EXCLUSIVE);
+ DataChecksumState->success = DATACHECKSUMWORKER_ABORTED;
+ LWLockRelease(DataChecksumWorkerLock);
ereport(DEBUG1,
errmsg("data checksum processing aborted in database OID %u",
dboid));
@@ -1669,7 +1669,7 @@ DataChecksumsWorkerMain(Datum arg)
list_free(CurrentTempTables);
#ifdef USE_INJECTION_POINTS
- if (IS_INJECTION_POINT_ATTACHED("datachecksumsworker-fake-temptable-wait"))
+ if (IS_INJECTION_POINT_ATTACHED("datachecksumworker-fake-temptable-wait"))
{
/* Make sure to just cause one retry */
if (!retried && numleft == 0)
@@ -1677,7 +1677,7 @@ DataChecksumsWorkerMain(Datum arg)
numleft = 1;
retried = true;
- INJECTION_POINT_CACHED("datachecksumsworker-fake-temptable-wait", NULL);
+ INJECTION_POINT_CACHED("datachecksumworker-fake-temptable-wait", NULL);
}
}
#endif
@@ -1706,9 +1706,9 @@ DataChecksumsWorkerMain(Datum arg)
if (aborted || abort_requested)
{
- LWLockAcquire(DataChecksumsWorkerLock, LW_EXCLUSIVE);
- DataChecksumState->success = DATACHECKSUMSWORKER_ABORTED;
- LWLockRelease(DataChecksumsWorkerLock);
+ LWLockAcquire(DataChecksumWorkerLock, LW_EXCLUSIVE);
+ DataChecksumState->success = DATACHECKSUMWORKER_ABORTED;
+ LWLockRelease(DataChecksumWorkerLock);
ereport(LOG,
errmsg("data checksum processing aborted in database OID %u",
dboid));
@@ -1721,7 +1721,7 @@ DataChecksumsWorkerMain(Datum arg)
/* worker done */
pgstat_progress_end_command();
- LWLockAcquire(DataChecksumsWorkerLock, LW_EXCLUSIVE);
- DataChecksumState->success = DATACHECKSUMSWORKER_SUCCESSFUL;
- LWLockRelease(DataChecksumsWorkerLock);
+ LWLockAcquire(DataChecksumWorkerLock, LW_EXCLUSIVE);
+ DataChecksumState->success = DATACHECKSUMWORKER_SUCCESSFUL;
+ LWLockRelease(DataChecksumWorkerLock);
}
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 90c7c4528e8..0debcb18991 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -3010,8 +3010,8 @@ PostmasterStateMachine(void)
/* also add data checksums processes */
remainMask = btmask_add(remainMask,
- B_DATACHECKSUMSWORKER_LAUNCHER,
- B_DATACHECKSUMSWORKER_WORKER);
+ B_DATACHECKSUMWORKER_LAUNCHER,
+ B_DATACHECKSUMWORKER_WORKER);
/* All types should be included in targetMask or remainMask */
Assert((remainMask.mask | targetMask.mask) == BTYPE_MASK_ALL.mask);
diff --git a/src/backend/storage/ipc/procsignal.c b/src/backend/storage/ipc/procsignal.c
index 1397f65f67b..0e0c7acf0df 100644
--- a/src/backend/storage/ipc/procsignal.c
+++ b/src/backend/storage/ipc/procsignal.c
@@ -596,7 +596,7 @@ ProcessProcSignalBarrier(void)
case PROCSIGNAL_BARRIER_CHECKSUM_ON:
case PROCSIGNAL_BARRIER_CHECKSUM_INPROGRESS_OFF:
case PROCSIGNAL_BARRIER_CHECKSUM_OFF:
- processed = AbsorbDataChecksumsBarrier(type);
+ processed = AbsorbDataChecksumBarrier(type);
break;
}
diff --git a/src/backend/utils/activity/pgstat_backend.c b/src/backend/utils/activity/pgstat_backend.c
index 73461c9bca5..bccf6f27121 100644
--- a/src/backend/utils/activity/pgstat_backend.c
+++ b/src/backend/utils/activity/pgstat_backend.c
@@ -381,8 +381,8 @@ pgstat_tracks_backend_bktype(BackendType bktype)
case B_CHECKPOINTER:
case B_IO_WORKER:
case B_STARTUP:
- case B_DATACHECKSUMSWORKER_LAUNCHER:
- case B_DATACHECKSUMSWORKER_WORKER:
+ case B_DATACHECKSUMWORKER_LAUNCHER:
+ case B_DATACHECKSUMWORKER_WORKER:
return false;
case B_AUTOVAC_WORKER:
diff --git a/src/backend/utils/activity/pgstat_io.c b/src/backend/utils/activity/pgstat_io.c
index 13a5d8e6440..49f0466040b 100644
--- a/src/backend/utils/activity/pgstat_io.c
+++ b/src/backend/utils/activity/pgstat_io.c
@@ -362,8 +362,8 @@ pgstat_tracks_io_bktype(BackendType bktype)
case B_LOGGER:
return false;
- case B_DATACHECKSUMSWORKER_LAUNCHER:
- case B_DATACHECKSUMSWORKER_WORKER:
+ case B_DATACHECKSUMWORKER_LAUNCHER:
+ case B_DATACHECKSUMWORKER_WORKER:
case B_AUTOVAC_LAUNCHER:
case B_AUTOVAC_WORKER:
case B_BACKEND:
diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index 7ffc808073a..42a0e896f1d 100644
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -846,7 +846,7 @@ InitializeSessionUserIdStandalone(void)
*/
Assert(!IsUnderPostmaster || AmAutoVacuumWorkerProcess() ||
AmLogicalSlotSyncWorkerProcess() || AmBackgroundWorkerProcess() ||
- AmDataChecksumsWorkerProcess());
+ AmDataChecksumWorkerProcess());
/* call only once */
Assert(!OidIsValid(AuthenticatedUserId));
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index 2460e550f96..9acd984da42 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -921,7 +921,7 @@ InitPostgres(const char *in_dbname, Oid dboid,
errhint("You should immediately run CREATE USER \"%s\" SUPERUSER;.",
username != NULL ? username : "postgres")));
}
- else if (AmBackgroundWorkerProcess() || AmDataChecksumsWorkerProcess())
+ else if (AmBackgroundWorkerProcess() || AmDataChecksumWorkerProcess())
{
if (username == NULL && !OidIsValid(useroid))
{
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 7de0a115402..bc538ac212e 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -370,8 +370,8 @@ typedef enum BackendType
B_WAL_SUMMARIZER,
B_WAL_WRITER,
- B_DATACHECKSUMSWORKER_LAUNCHER,
- B_DATACHECKSUMSWORKER_WORKER,
+ B_DATACHECKSUMWORKER_LAUNCHER,
+ B_DATACHECKSUMWORKER_WORKER,
/*
* Logger is not connected to shared memory and does not have a PGPROC
@@ -398,9 +398,9 @@ extern PGDLLIMPORT BackendType MyBackendType;
#define AmWalSummarizerProcess() (MyBackendType == B_WAL_SUMMARIZER)
#define AmWalWriterProcess() (MyBackendType == B_WAL_WRITER)
#define AmIoWorkerProcess() (MyBackendType == B_IO_WORKER)
-#define AmDataChecksumsWorkerProcess() \
- (MyBackendType == B_DATACHECKSUMSWORKER_LAUNCHER || \
- MyBackendType == B_DATACHECKSUMSWORKER_WORKER)
+#define AmDataChecksumWorkerProcess() \
+ (MyBackendType == B_DATACHECKSUMWORKER_LAUNCHER || \
+ MyBackendType == B_DATACHECKSUMWORKER_WORKER)
#define AmSpecialWorkerProcess() \
(AmAutoVacuumLauncherProcess() || \
diff --git a/src/include/postmaster/datachecksum_state.h b/src/include/postmaster/datachecksum_state.h
index 2a1ae10d55d..4426f9dc622 100644
--- a/src/include/postmaster/datachecksum_state.h
+++ b/src/include/postmaster/datachecksum_state.h
@@ -17,12 +17,12 @@
#include "storage/procsignal.h"
-/* Possible operations the DataChecksumsWorker can perform */
-typedef enum DataChecksumsWorkerOperation
+/* Possible operations the DataChecksumWorker can perform */
+typedef enum DataChecksumWorkerOperation
{
ENABLE_DATACHECKSUMS,
DISABLE_DATACHECKSUMS,
-} DataChecksumsWorkerOperation;
+} DataChecksumWorkerOperation;
/*
* Possible states for a database entry which has been processed. Exported
@@ -30,25 +30,25 @@ typedef enum DataChecksumsWorkerOperation
*/
typedef enum
{
- DATACHECKSUMSWORKER_SUCCESSFUL = 0,
- DATACHECKSUMSWORKER_ABORTED,
- DATACHECKSUMSWORKER_FAILED,
- DATACHECKSUMSWORKER_DROPDB,
-} DataChecksumsWorkerResult;
+ DATACHECKSUMWORKER_SUCCESSFUL = 0,
+ DATACHECKSUMWORKER_ABORTED,
+ DATACHECKSUMWORKER_FAILED,
+ DATACHECKSUMWORKER_DROPDB,
+} DataChecksumWorkerResult;
/* Prototypes for data checksum state manipulation */
-bool AbsorbDataChecksumsBarrier(ProcSignalBarrierType barrier);
-void EmitAndWaitDataChecksumsBarrier(uint32 state);
+bool AbsorbDataChecksumBarrier(ProcSignalBarrierType barrier);
+void EmitAndWaitDataChecksumBarrier(uint32 state);
/* Prototypes for data checksum background worker */
/* Start the background processes for enabling or disabling checksums */
-void StartDataChecksumsWorkerLauncher(DataChecksumsWorkerOperation op,
- int cost_delay,
- int cost_limit);
+void StartDataChecksumWorkerLauncher(DataChecksumWorkerOperation op,
+ int cost_delay,
+ int cost_limit);
/* Background worker entrypoints */
-void DataChecksumsWorkerLauncherMain(Datum arg);
-void DataChecksumsWorkerMain(Datum arg);
+void DataChecksumWorkerLauncherMain(Datum arg);
+void DataChecksumWorkerMain(Datum arg);
#endif /* DATACHECKSUM_STATE_H */
diff --git a/src/include/postmaster/proctypelist.h b/src/include/postmaster/proctypelist.h
index b3477e6f17a..ec84860a613 100644
--- a/src/include/postmaster/proctypelist.h
+++ b/src/include/postmaster/proctypelist.h
@@ -38,8 +38,8 @@ PG_PROCTYPE(B_BACKEND, "backend", gettext_noop("client backend"), BackendMain, t
PG_PROCTYPE(B_BG_WORKER, "bgworker", gettext_noop("background worker"), BackgroundWorkerMain, true)
PG_PROCTYPE(B_BG_WRITER, "bgwriter", gettext_noop("background writer"), BackgroundWriterMain, true)
PG_PROCTYPE(B_CHECKPOINTER, "checkpointer", gettext_noop("checkpointer"), CheckpointerMain, true)
-PG_PROCTYPE(B_DATACHECKSUMSWORKER_LAUNCHER, "checksums", gettext_noop("datachecksum launcher"), NULL, false)
-PG_PROCTYPE(B_DATACHECKSUMSWORKER_WORKER, "checksums", gettext_noop("datachecksum worker"), NULL, false)
+PG_PROCTYPE(B_DATACHECKSUMWORKER_LAUNCHER, "checksums", gettext_noop("datachecksum launcher"), NULL, false)
+PG_PROCTYPE(B_DATACHECKSUMWORKER_WORKER, "checksums", gettext_noop("datachecksum worker"), NULL, false)
PG_PROCTYPE(B_DEAD_END_BACKEND, "backend", gettext_noop("dead-end client backend"), BackendMain, true)
PG_PROCTYPE(B_INVALID, "postmaster", gettext_noop("unrecognized"), NULL, false)
PG_PROCTYPE(B_IO_WORKER, "ioworker", gettext_noop("io worker"), IoWorkerMain, true)
diff --git a/src/include/storage/subsystemlist.h b/src/include/storage/subsystemlist.h
index 9ad619080be..e9bd1ce0681 100644
--- a/src/include/storage/subsystemlist.h
+++ b/src/include/storage/subsystemlist.h
@@ -84,7 +84,7 @@ PG_SHMEM_SUBSYSTEM(InjectionPointShmemCallbacks)
#endif
PG_SHMEM_SUBSYSTEM(WaitLSNShmemCallbacks)
PG_SHMEM_SUBSYSTEM(LogicalDecodingCtlShmemCallbacks)
-PG_SHMEM_SUBSYSTEM(DataChecksumsShmemCallbacks)
+PG_SHMEM_SUBSYSTEM(DataChecksumShmemCallbacks)
/* AIO subsystem. This delegates to the method-specific callbacks */
PG_SHMEM_SUBSYSTEM(AioShmemCallbacks)
--
2.47.3
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: A few message wording/formatting cleanup patches
2026-05-28 03:16 A few message wording/formatting cleanup patches Kyotaro Horiguchi <horikyota.ntt@gmail.com>
@ 2026-05-28 17:19 ` Daniel Gustafsson <daniel@yesql.se>
2026-05-28 17:23 ` Re: A few message wording/formatting cleanup patches Jacob Champion <jacob.champion@enterprisedb.com>
2026-05-28 19:04 ` Re: A few message wording/formatting cleanup patches Tomas Vondra <tomas@vondra.me>
1 sibling, 2 replies; 10+ messages in thread
From: Daniel Gustafsson @ 2026-05-28 17:19 UTC (permalink / raw)
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; +Cc: pgsql-hackers@lists.postgresql.org
> On 28 May 2026, at 05:16, Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote:
>
> Hello,
>
> While translating error messages into Japanese
Thank you so much for doing this!
Some comments on a few of your patches:
> 0002: 0002-Use-double-quotes-in-message.patch
>
> fe-protocol3.c has the following message containing backquotes:
>
>> libpq_append_conn_error(conn,
>> "server did not report the unsupported `_pq_.test_protocol_negotiation` parameter in its protocol negotiation message");
>
> but that doesn't seem to match our usual style. Nearby messages use
> double quotes instead.
Agreed. Shouldn't it also be adding the parameter as a %s to further match our style?
- libpq_append_conn_error(conn, "server did not report the unsupported `_pq_.test_protocol_negotiation` parameter in its protocol negotiation message");
+ libpq_append_conn_error(conn, "server did not report the unsupported \"%s\" parameter in its protocol negotiation message", "_pq_.test_protocol_negotiation");
> 0003: 0003-Add-missing-period-to-HINT-message.patch
>
> In be-secure-openssl.c, the following HINT message is missing a
> trailing period:
>
>> errhint("If ssl_sni is enabled then add configuration to "%s", else "%s"",
My bad, will fix. Reading the hint it's also not as helpful as it could be for
a hint I think, perhaps this would be better?
- errhint("If ssl_sni is enabled then add configuration to \"%s\", else \"%s\"",
+ errhint("If ssl_sni is enabled then add configuration to \"%s\", else configure SSL in \"%s\".",
> 0004: 0004-Use-singular-datachecksum-consistently-in-process-na.patch
>
> In datachecksum_state.c, the launcher process is referred to in two
> different ways: "datachecksum launcher" and "datachecksums
> launcher". Considering the worker process name, I think the former is
> probably the intended one, so this patch makes the naming consistent
> accordingly.
>
> That said, I can also imagine an interpretation where "datachecksums"
> was chosen intentionally to refer to the checksum feature or checksum
> set as a whole, so I'm not entirely sure whether this should be
> considered a real issue or just a stylistic inconsistency.
>
> Still, having both forms coexist seems somewhat error-prone in
> practice, especially when typing or searching symbol names.
I am clearly biased, or Stockholm syndromed, but DataChecksumsXXX was chosen
deliberately since it affects the feature which is exposed with the GUC
data_checksums. Renaming it does not improve clarity IMHO. The singular form
"checksum" is used where it refers to a single entity, like the cluster state
which can only be a single value.
It would however be an improvement to rename the "datachecksum launcher|worker"
cases you found to "datachecksums" since they are user facing.
- * This creates the list of databases for the datachecksumsworker workers to
+ * This creates the list of databases for the datachecksum workers to
This comment refers to a worker process of the type datachecksumsworker, the
proposed change makes that less clear IMO. That said, the original wording
isn't great so maybe "datachecksumsworker process" is better?
--
Daniel Gustafsson
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: A few message wording/formatting cleanup patches
2026-05-28 03:16 A few message wording/formatting cleanup patches Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-05-28 17:19 ` Re: A few message wording/formatting cleanup patches Daniel Gustafsson <daniel@yesql.se>
@ 2026-05-28 17:23 ` Jacob Champion <jacob.champion@enterprisedb.com>
2026-05-29 18:46 ` Re: A few message wording/formatting cleanup patches Jacob Champion <jacob.champion@enterprisedb.com>
1 sibling, 1 reply; 10+ messages in thread
From: Jacob Champion @ 2026-05-28 17:23 UTC (permalink / raw)
To: Daniel Gustafsson <daniel@yesql.se>; +Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; pgsql-hackers@lists.postgresql.org
On Thu, May 28, 2026 at 10:19 AM Daniel Gustafsson <daniel@yesql.se> wrote:
> > On 28 May 2026, at 05:16, Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote:
> >> libpq_append_conn_error(conn,
> >> "server did not report the unsupported `_pq_.test_protocol_negotiation` parameter in its protocol negotiation message");
> >
> > but that doesn't seem to match our usual style. Nearby messages use
> > double quotes instead.
>
> Agreed. Shouldn't it also be adding the parameter as a %s to further match our style?
>
> - libpq_append_conn_error(conn, "server did not report the unsupported `_pq_.test_protocol_negotiation` parameter in its protocol negotiation message");
> + libpq_append_conn_error(conn, "server did not report the unsupported \"%s\" parameter in its protocol negotiation message", "_pq_.test_protocol_negotiation");
+1, will fix.
Thanks!
--Jacob
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: A few message wording/formatting cleanup patches
2026-05-28 03:16 A few message wording/formatting cleanup patches Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-05-28 17:19 ` Re: A few message wording/formatting cleanup patches Daniel Gustafsson <daniel@yesql.se>
2026-05-28 17:23 ` Re: A few message wording/formatting cleanup patches Jacob Champion <jacob.champion@enterprisedb.com>
@ 2026-05-29 18:46 ` Jacob Champion <jacob.champion@enterprisedb.com>
0 siblings, 0 replies; 10+ messages in thread
From: Jacob Champion @ 2026-05-29 18:46 UTC (permalink / raw)
To: Daniel Gustafsson <daniel@yesql.se>; +Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; pgsql-hackers@lists.postgresql.org
On Thu, May 28, 2026 at 10:23 AM Jacob Champion
<jacob.champion@enterprisedb.com> wrote:
> +1, will fix.
(Pushed.)
--Jacob
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: A few message wording/formatting cleanup patches
2026-05-28 03:16 A few message wording/formatting cleanup patches Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-05-28 17:19 ` Re: A few message wording/formatting cleanup patches Daniel Gustafsson <daniel@yesql.se>
@ 2026-05-28 19:04 ` Tomas Vondra <tomas@vondra.me>
2026-05-29 20:10 ` Re: A few message wording/formatting cleanup patches Daniel Gustafsson <daniel@yesql.se>
1 sibling, 1 reply; 10+ messages in thread
From: Tomas Vondra @ 2026-05-28 19:04 UTC (permalink / raw)
To: Daniel Gustafsson <daniel@yesql.se>; Kyotaro Horiguchi <horikyota.ntt@gmail.com>; +Cc: pgsql-hackers@lists.postgresql.org
On 5/28/26 19:19, Daniel Gustafsson wrote:
>> On 28 May 2026, at 05:16, Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote:
>>
>> ...
>>
>> 0004: 0004-Use-singular-datachecksum-consistently-in-process-na.patch
>>
>> In datachecksum_state.c, the launcher process is referred to in two
>> different ways: "datachecksum launcher" and "datachecksums
>> launcher". Considering the worker process name, I think the former is
>> probably the intended one, so this patch makes the naming consistent
>> accordingly.
>>
>> That said, I can also imagine an interpretation where "datachecksums"
>> was chosen intentionally to refer to the checksum feature or checksum
>> set as a whole, so I'm not entirely sure whether this should be
>> considered a real issue or just a stylistic inconsistency.
>>
>> Still, having both forms coexist seems somewhat error-prone in
>> practice, especially when typing or searching symbol names.
>
> I am clearly biased, or Stockholm syndromed, but DataChecksumsXXX was chosen
> deliberately since it affects the feature which is exposed with the GUC
> data_checksums. Renaming it does not improve clarity IMHO. The singular form
> "checksum" is used where it refers to a single entity, like the cluster state
> which can only be a single value.
>
> It would however be an improvement to rename the "datachecksum launcher|worker"
> cases you found to "datachecksums" since they are user facing.
>
> - * This creates the list of databases for the datachecksumsworker workers to
> + * This creates the list of databases for the datachecksum workers to
>
> This comment refers to a worker process of the type datachecksumsworker, the
> proposed change makes that less clear IMO. That said, the original wording
> isn't great so maybe "datachecksumsworker process" is better?
>
My vote would be to use the plural "data checksums" almost everywhere,
except for the places that actually manipulates a single data checksum.
Because the feature is "data checksums" with GUC "data_checksums", we
manipulate all checksums in the cluster, and so on.
So it'd be "datachecksums_state.c" and "datachecksums launcher" (which
would make it more consistent with the existing injection points, named
"datachecksumsworker-launcher-..." etc.).
But this opinion comes from someone who accidentally named an internal
queueing library "queque" and had to live with that shame for years.
regards
--
Tomas Vondra
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: A few message wording/formatting cleanup patches
2026-05-28 03:16 A few message wording/formatting cleanup patches Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-05-28 17:19 ` Re: A few message wording/formatting cleanup patches Daniel Gustafsson <daniel@yesql.se>
2026-05-28 19:04 ` Re: A few message wording/formatting cleanup patches Tomas Vondra <tomas@vondra.me>
@ 2026-05-29 20:10 ` Daniel Gustafsson <daniel@yesql.se>
0 siblings, 0 replies; 10+ messages in thread
From: Daniel Gustafsson @ 2026-05-29 20:10 UTC (permalink / raw)
To: Tomas Vondra <tomas@vondra.me>; +Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; pgsql-hackers@lists.postgresql.org
> On 28 May 2026, at 21:04, Tomas Vondra <tomas@vondra.me> wrote:
>
> On 5/28/26 19:19, Daniel Gustafsson wrote:
>>> On 28 May 2026, at 05:16, Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote:
>>>
>>> ...
>>>
>>> 0004: 0004-Use-singular-datachecksum-consistently-in-process-na.patch
>>>
>>> In datachecksum_state.c, the launcher process is referred to in two
>>> different ways: "datachecksum launcher" and "datachecksums
>>> launcher". Considering the worker process name, I think the former is
>>> probably the intended one, so this patch makes the naming consistent
>>> accordingly.
>>>
>>> That said, I can also imagine an interpretation where "datachecksums"
>>> was chosen intentionally to refer to the checksum feature or checksum
>>> set as a whole, so I'm not entirely sure whether this should be
>>> considered a real issue or just a stylistic inconsistency.
>>>
>>> Still, having both forms coexist seems somewhat error-prone in
>>> practice, especially when typing or searching symbol names.
>>
>> I am clearly biased, or Stockholm syndromed, but DataChecksumsXXX was chosen
>> deliberately since it affects the feature which is exposed with the GUC
>> data_checksums. Renaming it does not improve clarity IMHO. The singular form
>> "checksum" is used where it refers to a single entity, like the cluster state
>> which can only be a single value.
>>
>> It would however be an improvement to rename the "datachecksum launcher|worker"
>> cases you found to "datachecksums" since they are user facing.
>>
>> - * This creates the list of databases for the datachecksumsworker workers to
>> + * This creates the list of databases for the datachecksum workers to
>>
>> This comment refers to a worker process of the type datachecksumsworker, the
>> proposed change makes that less clear IMO. That said, the original wording
>> isn't great so maybe "datachecksumsworker process" is better?
>
> My vote would be to use the plural "data checksums" almost everywhere,
> except for the places that actually manipulates a single data checksum.
> Because the feature is "data checksums" with GUC "data_checksums", we
> manipulate all checksums in the cluster, and so on.
>
> So it'd be "datachecksums_state.c" and "datachecksums launcher" (which
> would make it more consistent with the existing injection points, named
> "datachecksumsworker-launcher-..." etc.).
I've pushed the worker/launcher rename now. I held off on renaming the file
for now, I'm not entirely convinced that's an improvement.
--
Daniel Gustafsson
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: A few message wording/formatting cleanup patches
2026-05-28 03:16 A few message wording/formatting cleanup patches Kyotaro Horiguchi <horikyota.ntt@gmail.com>
@ 2026-06-01 11:48 ` Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
2026-06-02 04:43 ` Re: A few message wording/formatting cleanup patches Kyotaro Horiguchi <horikyota.ntt@gmail.com>
1 sibling, 1 reply; 10+ messages in thread
From: Ashutosh Bapat @ 2026-06-01 11:48 UTC (permalink / raw)
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; +Cc: pgsql-hackers@lists.postgresql.org
Hi Horiguchi-san,
On Thu, May 28, 2026 at 8:46 AM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:
>
> Hello,
>
> While translating error messages into Japanese, I came across some
> unusually formatted messages and messages that are difficult to
> translate naturally into Japanese.
>
> 0001: 0001-Make-propgraph-object-descriptions-translatable.patch
>
> getObjectDescription() currently builds the object name for
> PropgraphElementRelationId incrementally with appendStringInfo(), but
> that works poorly with Japanese because it effectively fixes the word
> order. We probably need to construct the whole name from a single
> format string instead.
>
Sorry for delay in review. Few comments
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.
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.
--
Best Wishes,
Ashutosh Bapat
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: A few message wording/formatting cleanup patches
2026-05-28 03:16 A few message wording/formatting cleanup patches Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-01 11:48 ` Re: A few message wording/formatting cleanup patches Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
@ 2026-06-02 04:43 ` Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-02 06:28 ` Re: A few message wording/formatting cleanup patches Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
0 siblings, 1 reply; 10+ messages in thread
From: Kyotaro Horiguchi @ 2026-06-02 04:43 UTC (permalink / raw)
To: ashutosh.bapat.oss@gmail.com; +Cc: pgsql-hackers@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: A few message wording/formatting cleanup patches
2026-05-28 03:16 A few message wording/formatting cleanup patches Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-01 11:48 ` Re: A few message wording/formatting cleanup patches Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
2026-06-02 04:43 ` Re: A few message wording/formatting cleanup patches Kyotaro Horiguchi <horikyota.ntt@gmail.com>
@ 2026-06-02 06:28 ` Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
2026-07-03 21:52 ` Re: A few message wording/formatting cleanup patches Peter Eisentraut <peter@eisentraut.org>
0 siblings, 1 reply; 10+ messages in thread
From: Ashutosh Bapat @ 2026-06-02 06:28 UTC (permalink / raw)
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; +Cc: pgsql-hackers@lists.postgresql.org
On Tue, Jun 2, 2026 at 10:13 AM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:
>
> 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.
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
Attachments:
[text/x-patch] v20260602-0009-Cosmetic-adjustments.patch (5.7K, ../../CAExHW5uDMr4x9=is8MwKzEzv3YVVTv_q36XtadMp=b5vEyJcRg@mail.gmail.com/2-v20260602-0009-Cosmetic-adjustments.patch)
download | inline diff:
From e12e3988856e22a889b518097bf5ae125a169114 Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
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 <ashutosh.bapat.oss@gmail.com>
---
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
[text/x-patch] v20260602-0008-Make-propgraph-object-descriptions-transla.patch (6.2K, ../../CAExHW5uDMr4x9=is8MwKzEzv3YVVTv_q36XtadMp=b5vEyJcRg@mail.gmail.com/3-v20260602-0008-Make-propgraph-object-descriptions-transla.patch)
download | inline diff:
From bd6802810ef76c45814f20d611c3d6a69dd754c5 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 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
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: A few message wording/formatting cleanup patches
2026-05-28 03:16 A few message wording/formatting cleanup patches Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-01 11:48 ` Re: A few message wording/formatting cleanup patches Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
2026-06-02 04:43 ` Re: A few message wording/formatting cleanup patches Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-02 06:28 ` Re: A few message wording/formatting cleanup patches Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
@ 2026-07-03 21:52 ` Peter Eisentraut <peter@eisentraut.org>
0 siblings, 0 replies; 10+ messages in thread
From: Peter Eisentraut @ 2026-07-03 21:52 UTC (permalink / raw)
To: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>; Kyotaro Horiguchi <horikyota.ntt@gmail.com>; +Cc: pgsql-hackers@lists.postgresql.org
On 02.06.26 08:28, Ashutosh Bapat wrote:
> 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
committed
^ permalink raw reply [nested|flat] 10+ messages in thread
end of thread, other threads:[~2026-07-03 21:52 UTC | newest]
Thread overview: 10+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-05-28 03:16 A few message wording/formatting cleanup patches Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-05-28 17:19 ` Daniel Gustafsson <daniel@yesql.se>
2026-05-28 17:23 ` Jacob Champion <jacob.champion@enterprisedb.com>
2026-05-29 18:46 ` Jacob Champion <jacob.champion@enterprisedb.com>
2026-05-28 19:04 ` Tomas Vondra <tomas@vondra.me>
2026-05-29 20:10 ` Daniel Gustafsson <daniel@yesql.se>
2026-06-01 11:48 ` Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
2026-06-02 04:43 ` Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2026-06-02 06:28 ` Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
2026-07-03 21:52 ` Peter Eisentraut <peter@eisentraut.org>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox