Minor _bt_checkkeys comment improvements

I noticed a trivial misspelling in the comment in 
_bt_tuple_before_array_skeys, and when I started to look at the 
surrounding comments, I noticed a few other things that could be 
improved. Patch attached, but let me go through the changes one by one:

> diff --git a/src/backend/access/nbtree/nbtreadpage.c b/src/backend/access/nbtree/nbtreadpage.c
> index 372426f6a63..7e1441d8dc0 100644
> --- a/src/backend/access/nbtree/nbtreadpage.c
> +++ b/src/backend/access/nbtree/nbtreadpage.c
> @@ -1120,13 +1120,13 @@ _bt_savepostingitem(BTScanOpaque so, int itemIndex, OffsetNumber offnum,
>  /*
>   * Test whether an indextuple satisfies all the scankey conditions.
>   *
> - * Return true if so, false if not.  If the tuple fails to pass the qual,
> + * Returns true if so, false if not.  If not,
>   * we also determine whether there's any need to continue the scan beyond
>   * this tuple, and set pstate.continuescan accordingly.  See comments for
>   * _bt_preprocess_keys() about how this is done.

The "if the tuple fails to pass the qual" is just another way to say 
"did not satisfy all the scankey conditions", which is what the first 
line says. It feels redundant, and actually a little confusing: it took 
me a moment to understand that it means the same thing.

>   *
>   * Forward scan callers can pass a high key tuple in the hopes of having
> - * us set *continuescan to false, and avoiding an unnecessary visit to
> + * us set pstate.continuescan to false, avoiding an unnecessary visit to
>   * the page to the right.
>   *
>   * Advances the scan's array keys when necessary for arrayKeys=true callers.

Some other functions take a "bool *continuescan" parameter, but this 
function sets it in 'pstate'.

> @@ -1136,7 +1136,7 @@ _bt_savepostingitem(BTScanOpaque so, int itemIndex, OffsetNumber offnum,
>   * Scans with array keys are required to set up page state that helps us with
>   * this.  The page's finaltup tuple (the page high key for a forward scan, or
>   * the page's first non-pivot tuple for a backward scan) must be set in
> - * pstate.finaltup ahead of the first call here for the page.  Set this to
> + * pstate.finaltup ahead of the first call here for the page.  Set it to
>   * NULL for rightmost page (or the leftmost page for backwards scans).
>   *
>   * scan: index scan descriptor (containing a search-type scankey)

This is debatable, but "it" feels more natural to me than "this".

> @@ -1178,7 +1178,7 @@ _bt_checkkeys(IndexScanDesc scan, BTReadPageState *pstate, bool arrayKeys,
>                                         dcontinuescan;
>                 int                     dikey = 0;
>  
> -               /* Pass arrayKeys=false to avoid array side-effects */
> +               /* Pass advancenonrequired=false to avoid array side-effects */
>                 dres = _bt_check_compare(scan, dir, tuple, tupnatts, tupdesc, false,
>                                                                  pstate->forcenonrequired, &dcontinuescan,
>                                                                  &dikey);

The _bt_check_compare argument is called "advancenonrequired". The local 
variable in _bt_checkkeys that is usually passed as the argument is 
called "arrayKeys", so there's some logic to calling it that here. But 
"advancenonrequired" seems more clear.

> @@ -1197,9 +1197,9 @@ _bt_checkkeys(IndexScanDesc scan, BTReadPageState *pstate, bool arrayKeys,
>  
>         /*
>          * Only one _bt_check_compare call is required in the common case where
> -        * there are no equality strategy array scan keys.  Otherwise we can only
> -        * accept _bt_check_compare's answer unreservedly when it didn't set
> -        * pstate.continuescan=false.
> +        * there are no equality strategy array scan keys.  With array keys, we
> +        * can only accept _bt_check_compare's answer unreservedly when it set
> +        * pstate.continuescan=true.
>          */
>         if (!arrayKeys || pstate->continuescan)
>                 return res;

Double negative: "didn't set to false" means "did set to true" in this 
context. And the sentence was a little difficult to parse in other ways 
too: it takes some effort to decipher that "otherwise" means that there 
are array keys, and "only accept unreservedly" sound like more negation. 
(I didn't change the "only accept unreservedly" part)

Are there other kind of array keys than "equality strategy array keys"?


> @@ -2035,18 +2035,18 @@ _bt_tuple_before_array_skeys(IndexScanDesc scan, ScanDirection dir,
>  /*
>   * Determine if a scan with array keys should skip over uninteresting tuples.
>   *
> - * This is a subroutine for _bt_checkkeys.  Called when _bt_readpage's linear
> - * search process (started after it finishes reading an initial group of
> - * matching tuples, used to locate the start of the next group of tuples
> - * matching the next set of required array keys) has already scanned an
> - * excessive number of tuples whose key space is "between arrays".
> + * This is a subroutine for _bt_checkkeys, called when _bt_readpage's linear
> + * search process has scanned an excessive number of tuples whose key space is
> + * "between arrays".  (The linear search process is started after _bt_readpage
> + * finishes reading an initial group of matching tuples.  It locates the start
> + * of the first group of tuples matching the next set of required array keys.)
>   *

Complicated sentence. By the time I reached the end paren, I had already 
forgotten what it started with. I tried to rearrange it so that it you 
don't need such a deep mental stack.

> - * When we perform look ahead successfully, we'll sets pstate.skip, which
> + * When look ahead is successful, we set pstate.skip which
>   * instructs _bt_readpage to skip ahead to that tuple next (could be past the
>   * end of the scan's leaf page).  Pages where the optimization is effective

"we'll sets" was a misspelling. I made it a little less verbose while at it.

>   * will generally still need to skip several times.  Each call here performs
>   * only a single "look ahead" comparison of a later tuple, whose distance from
> - * the current tuple's offset number is determined by applying heuristics.
> + * the current tuple is determined by heuristics.
>   */

Just to make it less verbose. This is debatable, because the new wording 
"distance from the current tuple", could be misunderstood to mean some 
measure of distance of the key values rather than the distance of the 
tuples on the page. But I think I'd still go for brevity here.

Note that I didn't pgindent the patch, to make it more clear what's 
changed. Should re-indent before pushing.

- Heikki