Horton

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix nbtree skip array transformation comments.

  1. Fix comments on _bt_skiparray_strat_increment/decrement

    Yugo Nagata <nagata@sraoss.co.jp> — 2025-12-30T10:01:45Z

    Hi,

The attached patch is a trivial change to fix the comments on
_bt_skiparray_strat_increment() and _bt_skiparray_strat_decrement() so
that they are consistent with the comments on _bt_skiparray_strat_adjust(). 
At least the comment on _bt_skiparray_strat_decrement() containts an
obvious typo, since it mentions converting the high_compare key instead of
the low_compare key.

Regards,
Yugo Nagata

-- 
Yugo Nagata <nagata@sraoss.co.jp>

  2. Re: Fix comments on _bt_skiparray_strat_increment/decrement

    Chao Li <li.evan.chao@gmail.com> — 2025-12-31T03:36:56Z

    
> On Dec 30, 2025, at 18:01, Yugo Nagata <nagata@sraoss.co.jp> wrote:
> 
> Hi,
> 
> The attached patch is a trivial change to fix the comments on
> _bt_skiparray_strat_increment() and _bt_skiparray_strat_decrement() so
> that they are consistent with the comments on _bt_skiparray_strat_adjust(). 
> At least the comment on _bt_skiparray_strat_decrement() containts an
> obvious typo, since it mentions converting the high_compare key instead of
> the low_compare key.
> 
> Regards,
> Yugo Nagata
> 
> -- 
> Yugo Nagata <nagata@sraoss.co.jp>
> <fix_comments_on_bt_skiparray_strat_dec_inc_func.patch>

Good catch. Looks like a copy/paste mistake.

The code snippet prove the 2 functions' header comments are wrong:

```
/*
* Convert skip array's < low_compare key into a <= key
*/
static void
_bt_skiparray_strat_increment(IndexScanDesc scan, ScanKey arraysk,
BTArrayKeyInfo *array)
{
…
  if (RegProcedureIsValid(cmp_proc))
  {
    /* Transform > low_compare key into >= key */
    fmgr_info(cmp_proc, &low_compare->sk_func);
    low_compare->sk_argument = new_sk_argument;
    low_compare->sk_strategy = BTGreaterEqualStrategyNumber;
  }
```

And
```
/*
* Convert skip array's > low_compare key into a >= key
*/
static void
_bt_skiparray_strat_decrement(IndexScanDesc scan, ScanKey arraysk,
BTArrayKeyInfo *array)
{
…
  if (RegProcedureIsValid(cmp_proc))
  {
    /* Transform < high_compare key into <= key */
    fmgr_info(cmp_proc, &high_compare->sk_func);
    high_compare->sk_argument = new_sk_argument;
    high_compare->sk_strategy = BTLessEqualStrategyNumber;
  }
```

I also think we can delete “a” from the header comments. “into a >= key”, where “a” is an article (meaning one), but can be easily read as a variable name. The code comments don’t use “a” after “into”.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

  3. Re: Fix comments on _bt_skiparray_strat_increment/decrement

    Yugo Nagata <nagata@sraoss.co.jp> — 2025-12-31T06:57:24Z

    On Wed, 31 Dec 2025 11:36:56 +0800
Chao Li <li.evan.chao@gmail.com> wrote:

> 
> 
> > On Dec 30, 2025, at 18:01, Yugo Nagata <nagata@sraoss.co.jp> wrote:
> > 
> > Hi,
> > 
> > The attached patch is a trivial change to fix the comments on
> > _bt_skiparray_strat_increment() and _bt_skiparray_strat_decrement() so
> > that they are consistent with the comments on _bt_skiparray_strat_adjust(). 
> > At least the comment on _bt_skiparray_strat_decrement() containts an
> > obvious typo, since it mentions converting the high_compare key instead of
> > the low_compare key.
> > 
> > Regards,
> > Yugo Nagata
> > 
> > -- 
> > Yugo Nagata <nagata@sraoss.co.jp>
> > <fix_comments_on_bt_skiparray_strat_dec_inc_func.patch>
> 
> Good catch. Looks like a copy/paste mistake.
> 
> The code snippet prove the 2 functions' header comments are wrong:

Thank you for your reviewing.

> I also think we can delete “a” from the header comments. “into a >= key”, where “a” is an article (meaning one), but can be easily read as a variable name. The code comments don’t use “a” after “into”.

The existing comments are grammatically correct, but as you point out,
removing the "a" might make them less confusing. However, the comments in
_bt_skiparray_strat_adjust() and in _bt_preprocess_array_keys_final(), which
call this function, also use "a" after "into".

If we remove the "a" here, should we also update those comments for consistency?

-- 
Yugo Nagata <nagata@sraoss.co.jp>

  4. Re: Fix comments on _bt_skiparray_strat_increment/decrement

    Chao Li <li.evan.chao@gmail.com> — 2025-12-31T07:18:06Z

    
> On Dec 31, 2025, at 14:57, Yugo Nagata <nagata@sraoss.co.jp> wrote:
> 
> On Wed, 31 Dec 2025 11:36:56 +0800
> Chao Li <li.evan.chao@gmail.com> wrote:
> 
>> 
>> 
>>> On Dec 30, 2025, at 18:01, Yugo Nagata <nagata@sraoss.co.jp> wrote:
>>> 
>>> Hi,
>>> 
>>> The attached patch is a trivial change to fix the comments on
>>> _bt_skiparray_strat_increment() and _bt_skiparray_strat_decrement() so
>>> that they are consistent with the comments on _bt_skiparray_strat_adjust(). 
>>> At least the comment on _bt_skiparray_strat_decrement() containts an
>>> obvious typo, since it mentions converting the high_compare key instead of
>>> the low_compare key.
>>> 
>>> Regards,
>>> Yugo Nagata
>>> 
>>> -- 
>>> Yugo Nagata <nagata@sraoss.co.jp>
>>> <fix_comments_on_bt_skiparray_strat_dec_inc_func.patch>
>> 
>> Good catch. Looks like a copy/paste mistake.
>> 
>> The code snippet prove the 2 functions' header comments are wrong:
> 
> Thank you for your reviewing.
> 
>> I also think we can delete “a” from the header comments. “into a >= key”, where “a” is an article (meaning one), but can be easily read as a variable name. The code comments don’t use “a” after “into”.
> 
> The existing comments are grammatically correct, but as you point out,
> removing the "a" might make them less confusing. However, the comments in
> _bt_skiparray_strat_adjust() and in _bt_preprocess_array_keys_final(), which
> call this function, also use "a" after "into".
> 
> If we remove the "a" here, should we also update those comments for consistency?
> 
> -- 
> Yugo Nagata <nagata@sraoss.co.jp>

It’s probably better to leave them as-is and just fix the error, which should make the patch easier to get through.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

  5. Re: Fix comments on _bt_skiparray_strat_increment/decrement

    Yugo Nagata <nagata@sraoss.co.jp> — 2025-12-31T09:40:20Z

    On Wed, 31 Dec 2025 15:18:06 +0800
Chao Li <li.evan.chao@gmail.com> wrote:

> 
> 
> > On Dec 31, 2025, at 14:57, Yugo Nagata <nagata@sraoss.co.jp> wrote:
> > 
> > On Wed, 31 Dec 2025 11:36:56 +0800
> > Chao Li <li.evan.chao@gmail.com> wrote:
> > 
> >> 
> >> 
> >>> On Dec 30, 2025, at 18:01, Yugo Nagata <nagata@sraoss.co.jp> wrote:
> >>> 
> >>> Hi,
> >>> 
> >>> The attached patch is a trivial change to fix the comments on
> >>> _bt_skiparray_strat_increment() and _bt_skiparray_strat_decrement() so
> >>> that they are consistent with the comments on _bt_skiparray_strat_adjust(). 
> >>> At least the comment on _bt_skiparray_strat_decrement() containts an
> >>> obvious typo, since it mentions converting the high_compare key instead of
> >>> the low_compare key.
> >>> 
> >>> Regards,
> >>> Yugo Nagata
> >>> 
> >>> -- 
> >>> Yugo Nagata <nagata@sraoss.co.jp>
> >>> <fix_comments_on_bt_skiparray_strat_dec_inc_func.patch>
> >> 
> >> Good catch. Looks like a copy/paste mistake.
> >> 
> >> The code snippet prove the 2 functions' header comments are wrong:
> > 
> > Thank you for your reviewing.
> > 
> >> I also think we can delete “a” from the header comments. “into a >= key”, where “a” is an article (meaning one), but can be easily read as a variable name. The code comments don’t use “a” after “into”.
> > 
> > The existing comments are grammatically correct, but as you point out,
> > removing the "a" might make them less confusing. However, the comments in
> > _bt_skiparray_strat_adjust() and in _bt_preprocess_array_keys_final(), which
> > call this function, also use "a" after "into".
> > 
> > If we remove the "a" here, should we also update those comments for consistency?
> > 
> > -- 
> > Yugo Nagata <nagata@sraoss.co.jp>
> 
> It’s probably better to leave them as-is and just fix the error, which should make the patch easier to get through.

Agreed

Regards,
Yugo Nagata

-- 
Yugo Nagata <nagata@sraoss.co.jp>