Re: Adding skip scan (including MDAM style range skip scan) to nbtree

Masahiro Ikeda <ikedamsh@oss.nttdata.com>

From: Masahiro Ikeda <ikedamsh@oss.nttdata.com>
To: Peter Geoghegan <pg@bowt.ie>
Cc: Tomas Vondra <tomas@vondra.me>, Masahiro.Ikeda@nttdata.com, pgsql-hackers@lists.postgresql.org, Masao.Fujii@nttdata.com
Date: 2024-11-22T09:17:23Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. nbtree: Always set skipScan flag on rescan.

  2. meson: Build numeric.c with -ftree-vectorize.

  3. Fix "variable not found in subplan target lists" in semijoin de-duplication.

  4. Revert "nbtree: Remove useless row compare arg."

  5. nbtree: Remove useless row compare arg.

  6. Prevent premature nbtree array advancement.

  7. nbtree: tighten up array recheck rules.

  8. Avoid treating nonrequired nbtree keys as required.

  9. Adjust overstrong nbtree skip array assertion.

  10. Make NULL tuple values always advance skip arrays.

  11. Avoid extra index searches through preprocessing.

  12. Improve nbtree skip scan primitive scan scheduling.

  13. Further optimize nbtree search scan key comparisons.

  14. Add nbtree skip scan optimization.

  15. Improve nbtree array primitive scan scheduling.

  16. nbtree: Make BTMaxItemSize into object-like macro.

  17. Show index search count in EXPLAIN ANALYZE, take 2.

  18. Make parallel nbtree index scans use an LWLock.

  19. Show index search count in EXPLAIN ANALYZE.

  20. Avoid nbtree parallel scan currPos confusion.

  21. nbtree: Remove useless 'strat' local variable.

  22. Normalize nbtree truncated high key array behavior.

  23. Refactor handling of nbtree array redundancies.

  24. Fix nbtree pgstats accounting with parallel scans.

  25. Avoid parallel nbtree index scan hangs with SAOPs.

  26. Show Parallel Bitmap Heap Scan worker stats in EXPLAIN ANALYZE

  27. Enhance nbtree ScalarArrayOp execution.

  28. Skip checking of scan keys required for directional scan in B-tree

  29. Instead of using a numberOfRequiredKeys count to distinguish required

> On 2024-11-21 17:47, Masahiro Ikeda wrote:
>> On 2024-11-21 04:40, Peter Geoghegan wrote:
>> diff --git a/src/backend/access/nbtree/nbtutils.c
>> b/src/backend/access/nbtree/nbtutils.c
>> index b70b58e0c..ddae5f2a1 100644
>> --- a/src/backend/access/nbtree/nbtutils.c
>> +++ b/src/backend/access/nbtree/nbtutils.c
>> @@ -3640,7 +3640,7 @@ _bt_advance_array_keys(IndexScanDesc scan,
>> BTReadPageState *pstate,
>>       * for skip scan, and stop maintaining the scan's skip arrays 
>> until we
>>       * reach the page's finaltup, if any.
>>       */
>> -    if (has_skip_array && beyond_end_advance &&
>> +    if (has_skip_array && !all_required_satisfied &&
>>          !has_required_opposite_direction_skip && pstate->finaltup)
>>          pstate->beyondskip = true;
>> 
>> However, a small number of my test cases now fail. And (I assume) this
>> approach has certain downsides on leaf pages where we're now too quick
>> to stop maintaining skip arrays.
> 
> Since I've built with the above change and executed make check, I found
> that there is an assertion error, which may not be related to what you 
> pointed
> out.
> 
> * the reproducible simple query (you can see the original query in
> opr_sanity.sql).
>   select * from pg_proc
> 	where proname in (
> 	  'lo_lseek64',
> 	  'lo_truncate',
> 	  'lo_truncate64')
> 	and pronamespace = 11;
> 
> * the assertion error
>   TRAP: failed Assert("sktrig_required && required"), File:
> "nbtutils.c", Line: 3375, PID: 362411
> 
> While investigating the error, I thought we might need to consider
> whether key->sk_flags does not have SK_BT_SKIP. The assertion error
> occurs because
> requiredSameDir doesn't become true since proname does not have 
> SK_BT_SKIP.
> 
> +		if (beyondskip)
> +		{
> +			/*
> +			 * "Beyond end advancement" skip scan optimization.
> +			 *
> +			 * Just skip over any skip array scan keys.  Treat all other scan
> +			 * keys as not required for the scan to continue.
> +			 */
> +			Assert(!prechecked);
> +
> +			if (key->sk_flags & SK_BT_SKIP)
> +				continue;
> +		}
> +		else if (((key->sk_flags & SK_BT_REQFWD) && 
> ScanDirectionIsForward(dir)) ||
> +				 ((key->sk_flags & SK_BT_REQBKWD) && 
> ScanDirectionIsBackward(dir)))
>  			requiredSameDir = true;
> 

My previous investigation was incorrect, sorry. IIUC, 
_bt_check_compare()
should return false as soon as possible with continuescan=true after the
tuple fails the key check when beyondskip=true, rather than setting
requiredSameDir to true. Because it has already been triggered to 
perform
a full index scan for the page.

Though the change fixes the assertion error in 'make check', there are 
still
cases where the number of return values is incorrect. I would also like 
to
continue investigating.

Regards,
-- 
Masahiro Ikeda
NTT DATA CORPORATION