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


Reply via email to