Обсуждение: fix ancient typo in transformRelOptions()
I noticed that this function has a "namspace" parameter. The attached patch adds the missing 'e'. -- nathan
Вложения
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