Thread: [PATCH] Simple typos fix

Hi everybody,
   thanks a lot for your work.

   This is just a stupid patch to fix some typos.
   Thanks a lot to Magnus for the review.

   You can see it also on GitHub,¹ if you prefer, or
   apply it on top of today latest GIT.²

   It passed "make check" on Linux.

Ciao,
Gelma

---

   ¹ https://github.com/Gelma/postgres/commit/6c59961f91b89f55b103c57fffa94308dc29546a
   ² commit: d5ec46bf224d2ea1b010b2bc10a65e44d4456553

Thanks for finding these ; I think a few hunks are false positives and should
be removed.  A few more are debatable and could be correct either way:

Kazakstan
alloced - not an English word, but a technical one;
delink - "unlink" is better for the filesystem operation, but I don't think "delink" is wrong for a list operation.
dependees (?)
This'd
define'd

On Tue, May 28, 2019 at 08:17:18PM +0200, Andrea Gelmini wrote:
> diff --git a/contrib/amcheck/verify_nbtree.c b/contrib/amcheck/verify_nbtree.c
> index de0a98f6d9..ff13b0c9e7 100644
> --- a/contrib/amcheck/verify_nbtree.c
> +++ b/contrib/amcheck/verify_nbtree.c
> @@ -1278,7 +1278,7 @@ bt_right_page_check_scankey(BtreeCheckState *state)
>       * Routines like _bt_search() don't require *any* page split interlock
>       * when descending the tree, including something very light like a buffer
>       * pin. That's why it's okay that we don't either.  This avoidance of any
> -     * need to "couple" buffer locks is the raison d' etre of the Lehman & Yao
> +     * need to "couple" buffer locks is the reason d'etre of the Lehman & Yao

I think this is wrong.  The French phase is "raison d'etre".

> diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
> index e7c32f2a13..20bb928016 100644
> --- a/src/backend/replication/logical/reorderbuffer.c
> +++ b/src/backend/replication/logical/reorderbuffer.c
> @@ -2279,7 +2279,7 @@ ReorderBufferSerializeTXN(ReorderBuffer *rb, ReorderBufferTXN *txn)
>  
>          /*
>           * store in segment in which it belongs by start lsn, don't split over
> -         * multiple segments tho
> +         * multiple segments to

I think this is wrong.  It should say "though".  Or perhaps:
                 * store at segment to which its start lsn belongs, but don't split over
                 * multiple segments

> diff --git a/src/backend/utils/cache/relmapper.c b/src/backend/utils/cache/relmapper.c
> index 3b4f21bc54..403435df52 100644
> --- a/src/backend/utils/cache/relmapper.c
> +++ b/src/backend/utils/cache/relmapper.c
> @@ -146,7 +146,7 @@ static void perform_relmap_update(bool shared, const RelMapFile *updates);
>  /*
>   * RelationMapOidToFilenode
>   *
> - * The raison d' etre ... given a relation OID, look up its filenode.
> + * The reason d'etre... given a relation OID, look up its filenode.

Wrong

> @@ -907,7 +907,7 @@ write_relmap_file(bool shared, RelMapFile *newmap,
>       * Make sure that the files listed in the map are not deleted if the outer
>       * transaction aborts.  This had better be within the critical section
>       * too: it's not likely to fail, but if it did, we'd arrive at transaction
> -     * abort with the files still vulnerable.  PANICing will leave things in a
> +     * abort with the files still vulnerable.  Panicking will leave things in a

Wrong ?

Thanks,
Justin

On Sun, Jun 02, 2019 at 04:42:57PM -0500, Justin Pryzby wrote:
> Thanks for finding these ; I think a few hunks are false positives and should
> be removed.

Yes, some of them are the changes in imath.c and snowball/, which we
include in Postgres but in reality are independent projects, so these
should be fixed in upstream instead, and Postgres will include those
fixes when merging with newer versions.  If we were to fix those
issues ourselves, then we would likely create conflicts when merging
with newer versions of the upstream modules.

> A few more are debatable and could be correct either way:
>
> alloced - not an English word, but a technical one;

Indeed.  The current wording is fine by me.

> delink - "unlink" is better for the filesystem operation, but I
> don't think "delink" is wrong for a list operation.
> dependees (?)

These terms could be used in programming.

> This'd
> define'd

Don't think it is much of a big deal to keep these as well.
"invokable" can be used in programming, and "cachable" is an alternate
spelling of "cacheable" based on some research.

> On Tue, May 28, 2019 at 08:17:18PM +0200, Andrea Gelmini wrote:
>> diff --git a/contrib/amcheck/verify_nbtree.c b/contrib/amcheck/verify_nbtree.c
>> index de0a98f6d9..ff13b0c9e7 100644
>> --- a/contrib/amcheck/verify_nbtree.c
>> +++ b/contrib/amcheck/verify_nbtree.c
>> @@ -1278,7 +1278,7 @@ bt_right_page_check_scankey(BtreeCheckState *state)
>>       * Routines like _bt_search() don't require *any* page split interlock
>>       * when descending the tree, including something very light like a buffer
>>       * pin. That's why it's okay that we don't either.  This avoidance of any
>> -     * need to "couple" buffer locks is the raison d' etre of the Lehman & Yao
>> +     * need to "couple" buffer locks is the reason d'etre of the Lehman & Yao
>
> I think this is wrong.  The French phase is "raison d'etre".

French here.  Note that an accent is missing on the first 'e' (être)
but we don't want non-ASCII characters in the code.  So the current
wording is fine in my opinion.

> I think this is wrong.  It should say "though".  Or perhaps:
>                  * store at segment to which its start lsn belongs, but don't split over
>                  * multiple segments

I would replace it by "though", "tho" is not incorrect tho ;)

>> @@ -907,7 +907,7 @@ write_relmap_file(bool shared, RelMapFile *newmap,
>>       * Make sure that the files listed in the map are not deleted if the outer
>>       * transaction aborts.  This had better be within the critical section
>>       * too: it's not likely to fail, but if it did, we'd arrive at transaction
>> -     * abort with the files still vulnerable.  PANICing will leave things in a
>> +     * abort with the files still vulnerable.  Panicking will leave things in a
>
> Wrong ?

Yes, the suggestion is wrong.  The comment refers to the elog level.

The original patch proposed 63 diffs.  After the false positives are
removed, 21 remain, which I have now committed.  You have done good
work in catching all these, by the way.  Thanks for taking the time to
do so.
--
Michael