fix ancient typo in transformRelOptions()
I noticed that this function has a "namspace" parameter. The attached
patch adds the missing 'e'.
--
nathan
Attachments:
v1-0001-fix-ancient-typo-in-transformRelOptions.patchtext/plain; charset=us-asciiDownload
From 3f1431517cee250f1280adca739de0d6b9c77080 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Tue, 5 Aug 2025 16:01:34 -0500
Subject: [PATCH v1 1/1] fix ancient typo in transformRelOptions()
---
src/backend/access/common/reloptions.c | 10 +++++-----
src/include/access/reloptions.h | 2 +-
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 594a657ea1a..852cd73f5da 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -1164,7 +1164,7 @@ add_local_string_reloption(local_relopts *relopts, const char *name,
* but we declare them as Datums to avoid including array.h in reloptions.h.
*/
Datum
-transformRelOptions(Datum oldOptions, List *defList, const char *namspace,
+transformRelOptions(Datum oldOptions, List *defList, const char *namespace,
const char *const validnsps[], bool acceptOidsOff, bool isReset)
{
Datum result;
@@ -1200,14 +1200,14 @@ transformRelOptions(Datum oldOptions, List *defList, const char *namspace,
int kw_len;
/* ignore if not in the same namespace */
- if (namspace == NULL)
+ if (namespace == NULL)
{
if (def->defnamespace != NULL)
continue;
}
else if (def->defnamespace == NULL)
continue;
- else if (strcmp(def->defnamespace, namspace) != 0)
+ else if (strcmp(def->defnamespace, namespace) != 0)
continue;
kw_len = strlen(def->defname);
@@ -1277,14 +1277,14 @@ transformRelOptions(Datum oldOptions, List *defList, const char *namspace,
}
/* ignore if not in the same namespace */
- if (namspace == NULL)
+ if (namespace == NULL)
{
if (def->defnamespace != NULL)
continue;
}
else if (def->defnamespace == NULL)
continue;
- else if (strcmp(def->defnamespace, namspace) != 0)
+ else if (strcmp(def->defnamespace, namespace) != 0)
continue;
/*
diff --git a/src/include/access/reloptions.h b/src/include/access/reloptions.h
index dfbb4c85460..320f68ead8f 100644
--- a/src/include/access/reloptions.h
+++ b/src/include/access/reloptions.h
@@ -233,7 +233,7 @@ extern void add_local_string_reloption(local_relopts *relopts, const char *name,
fill_string_relopt filler, int offset);
extern Datum transformRelOptions(Datum oldOptions, List *defList,
- const char *namspace, const char *const validnsps[],
+ const char *namespace, const char *const validnsps[],
bool acceptOidsOff, bool isReset);
extern List *untransformRelOptions(Datum options);
extern bytea *extractRelOptions(HeapTuple tuple, TupleDesc tupdesc,
--
2.39.5 (Apple Git-154)
On 2025-Aug-05, Nathan Bossart wrote:
I noticed that this function has a "namspace" parameter. The attached
patch adds the missing 'e'.
I have vague recollections of it being this way because of "namespace"
being a C++ keyword. (commit is 3a5b77371522, but I don't have energy
to look for the discussion right now.)
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"We’ve narrowed the problem down to the customer’s pants being in a situation
of vigorous combustion" (Robert Haas, Postgres expert extraordinaire)
=?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de> writes:
On 2025-Aug-05, Nathan Bossart wrote:
I noticed that this function has a "namspace" parameter. The attached
patch adds the missing 'e'.
I have vague recollections of it being this way because of "namespace"
being a C++ keyword. (commit is 3a5b77371522, but I don't have energy
to look for the discussion right now.)
Oh, duh -- if you try cpluspluscheck you'll find it's unhappy.
I didn't locate the discussion of "namspace" either, but I did
notice that de160e2c0 used "nameSpace" for the same purpose.
Maybe we should standardize on that? It looks less like a typo
than "namspace", for sure.
regards, tom lane
On 2025-Aug-05, Tom Lane wrote:
=?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de> writes:
On 2025-Aug-05, Nathan Bossart wrote:
I noticed that this function has a "namspace" parameter. The attached
patch adds the missing 'e'.I have vague recollections of it being this way because of "namespace"
being a C++ keyword. (commit is 3a5b77371522, but I don't have energy
to look for the discussion right now.)Oh, duh -- if you try cpluspluscheck you'll find it's unhappy.
I didn't locate the discussion of "namspace" either, but I did
notice that de160e2c0 used "nameSpace" for the same purpose.
Maybe we should standardize on that? It looks less like a typo
than "namspace", for sure.
WFM.
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"No deja de ser humillante para una persona de ingenio saber
que no hay tonto que no le pueda enseñar algo." (Jean B. Say)
On Tue, Aug 05, 2025 at 06:10:54PM -0400, Tom Lane wrote:
=?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de> writes:
On 2025-Aug-05, Nathan Bossart wrote:
I noticed that this function has a "namspace" parameter. The attached
patch adds the missing 'e'.I have vague recollections of it being this way because of "namespace"
being a C++ keyword. (commit is 3a5b77371522, but I don't have energy
to look for the discussion right now.)Oh, duh -- if you try cpluspluscheck you'll find it's unhappy.
I didn't locate the discussion of "namspace" either, but I did
notice that de160e2c0 used "nameSpace" for the same purpose.
Maybe we should standardize on that? It looks less like a typo
than "namspace", for sure.
That explains it. I didn't find any past discussions about this particular
name, but commit de160e2 was being developed concurrently. I'll use
nameSpace instead.
--
nathan
On Wed, Aug 6, 2025 at 2:33 AM Nathan Bossart <nathandbossart@gmail.com> wrote:
I noticed that this function has a "namspace" parameter. The attached
patch adds the missing 'e'.
:)
Nobody seems to have been troubled by it for so long. Would fixing it
now create back-patching problems. Probably not since that code is old
enough to have less chances of bugs. So +1.
--
Best Wishes,
Ashutosh Bapat
On Tue, Aug 05, 2025 at 09:20:09PM -0500, Nathan Bossart wrote:
That explains it. I didn't find any past discussions about this particular
name, but commit de160e2 was being developed concurrently. I'll use
nameSpace instead.
Committed.
--
nathan
On Wed, Aug 06, 2025 at 12:09:28PM -0500, Nathan Bossart wrote:
On Tue, Aug 05, 2025 at 09:20:09PM -0500, Nathan Bossart wrote:
That explains it. I didn't find any past discussions about this particular
name, but commit de160e2 was being developed concurrently. I'll use
nameSpace instead.Committed.
Any thoughts on back-patching this? It's entirely cosmetic and could help
avoid some back-patching pain down the road. I originally chose not to
back-patch because it's not a bug and "namspace" has been there for a very
long time, but now I'm having second thoughts...
--
nathan
On 06.08.25 20:06, Nathan Bossart wrote:
On Wed, Aug 06, 2025 at 12:09:28PM -0500, Nathan Bossart wrote:
On Tue, Aug 05, 2025 at 09:20:09PM -0500, Nathan Bossart wrote:
That explains it. I didn't find any past discussions about this particular
name, but commit de160e2 was being developed concurrently. I'll use
nameSpace instead.Committed.
Any thoughts on back-patching this? It's entirely cosmetic and could help
avoid some back-patching pain down the road. I originally chose not to
back-patch because it's not a bug and "namspace" has been there for a very
long time, but now I'm having second thoughts...
I'd leave it alone for now. We could still choose to backpatch this one
if we end up backpatching something that goes on top of it.
Nathan Bossart <nathandbossart@gmail.com> writes:
Any thoughts on back-patching this? It's entirely cosmetic and could help
avoid some back-patching pain down the road. I originally chose not to
back-patch because it's not a bug and "namspace" has been there for a very
long time, but now I'm having second thoughts...
AFAICS, it could only cause back-patching pain if we were to
back-patch something that changes the signature of
transformRelOptions(), which seems mighty unlikely.
So I wouldn't bother.
regards, tom lane