v33-0005-DEBUG-Add-skip-scan-disable-GUCs.patch

From 545412ed16f77e0ad73295a28123c6ea4fd28a7f Mon Sep 17 00:00:00 2001
From: Peter Geoghegan <pg@bowt.ie>
Date: Sat, 18 Jan 2025 10:54:44 -0500
Subject: [PATCH v33 5/5] DEBUG: Add skip scan disable GUCs.

---
 src/include/access/nbtree.h                   |  5 +++
 src/backend/access/nbtree/nbtpreprocesskeys.c | 37 +++++++++++++++++++
 src/backend/access/nbtree/nbtutils.c          |  3 ++
 src/backend/utils/misc/guc_tables.c           | 34 +++++++++++++++++
 4 files changed, 79 insertions(+)

diff --git a/src/include/access/nbtree.h b/src/include/access/nbtree.h
index 02da56995..ca4b6ead4 100644
--- a/src/include/access/nbtree.h
+++ b/src/include/access/nbtree.h
@@ -1178,6 +1178,11 @@ typedef struct BTOptions
 #define PROGRESS_BTREE_PHASE_PERFORMSORT_2				4
 #define PROGRESS_BTREE_PHASE_LEAF_LOAD					5
 
+/* GUC parameters (just a temporary convenience for reviewers) */
+extern PGDLLIMPORT int skipscan_prefix_cols;
+extern PGDLLIMPORT bool skipscan_skipsupport_enabled;
+extern PGDLLIMPORT bool skipscan_iprefix_enabled;
+
 /*
  * external entry points for btree, in nbtree.c
  */
diff --git a/src/backend/access/nbtree/nbtpreprocesskeys.c b/src/backend/access/nbtree/nbtpreprocesskeys.c
index 0947eb0a6..6501ef620 100644
--- a/src/backend/access/nbtree/nbtpreprocesskeys.c
+++ b/src/backend/access/nbtree/nbtpreprocesskeys.c
@@ -21,6 +21,33 @@
 #include "utils/lsyscache.h"
 #include "utils/memutils.h"
 
+/*
+ * GUC parameters (temporary convenience for reviewers).
+ *
+ * To disable all skipping, set skipscan_prefix_cols=0.  Otherwise set it to
+ * the attribute number that you wish to make the last attribute number that
+ * we can add a skip scan key for.  For example, skipscan_prefix_cols=1 makes
+ * an index scan with qual "WHERE b = 1 AND d = 42" generate a skip scan key
+ * on the column 'a' (which is attnum 1) only, preventing us from adding one
+ * for the column 'c'.  And so only the scan key on 'b' (not the one on 'd')
+ * gets marked required within _bt_preprocess_keys -- there is no 'c' skip
+ * array to "anchor the required-ness" of 'b' through 'c' into 'd'.
+ */
+int			skipscan_prefix_cols = INDEX_MAX_KEYS;
+
+/*
+ * skipscan_skipsupport_enabled can be used to avoid using skip support.  Used
+ * to quantify the performance benefit that comes from having dedicated skip
+ * support, with a given opclass and test query.
+ */
+bool		skipscan_skipsupport_enabled = true;
+
+/*
+ * skipscan_iprefix_enabled can be used to disable optimizations used when the
+ * maintenance overhead of skip arrays stops paying for itself
+ */
+bool		skipscan_iprefix_enabled = true;
+
 typedef struct BTScanKeyPreproc
 {
 	ScanKey		inkey;
@@ -1650,6 +1677,10 @@ _bt_preprocess_array_keys(IndexScanDesc scan, int *new_numberOfKeys)
 			so->arrayKeys[numArrayKeys].low_compare = NULL; /* for now */
 			so->arrayKeys[numArrayKeys].high_compare = NULL;	/* for now */
 
+			/* Temporary testing GUC can disable the use of skip support */
+			if (!skipscan_skipsupport_enabled)
+				so->arrayKeys[numArrayKeys].sksup = NULL;
+
 			/*
 			 * We'll need a 3-way ORDER proc.  Set that up now.
 			 */
@@ -2175,6 +2206,12 @@ _bt_num_array_keys(IndexScanDesc scan, Oid *skip_eq_ops, int *numSkipArrayKeys)
 		if (attno_has_rowcompare)
 			break;
 
+		/*
+		 * Apply temporary testing GUC that can be used to disable skipping
+		 */
+		if (attno_inkey > skipscan_prefix_cols)
+			break;
+
 		/*
 		 * Now consider next attno_inkey (or keep going if this is an
 		 * additional scan key against the same attribute)
diff --git a/src/backend/access/nbtree/nbtutils.c b/src/backend/access/nbtree/nbtutils.c
index 2d060e185..38f79983f 100644
--- a/src/backend/access/nbtree/nbtutils.c
+++ b/src/backend/access/nbtree/nbtutils.c
@@ -2505,6 +2505,9 @@ _bt_set_startikey(IndexScanDesc scan, BTReadPageState *pstate)
 	if (so->numberOfKeys == 0)
 		return;
 
+	if (!skipscan_iprefix_enabled)
+		return;
+
 	/* minoff is an offset to the lowest non-pivot tuple on the page */
 	iid = PageGetItemId(pstate->page, pstate->minoff);
 	firsttup = (IndexTuple) PageGetItem(pstate->page, iid);
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 4eaeca89f..eb46c24c4 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -28,6 +28,7 @@
 
 #include "access/commit_ts.h"
 #include "access/gin.h"
+#include "access/nbtree.h"
 #include "access/slru.h"
 #include "access/toast_compression.h"
 #include "access/twophase.h"
@@ -1788,6 +1789,28 @@ struct config_bool ConfigureNamesBool[] =
 	},
 #endif
 
+	/* XXX Remove before commit */
+	{
+		{"skipscan_skipsupport_enabled", PGC_SUSET, DEVELOPER_OPTIONS,
+			NULL, NULL,
+			GUC_NOT_IN_SAMPLE
+		},
+		&skipscan_skipsupport_enabled,
+		true,
+		NULL, NULL, NULL
+	},
+
+	/* XXX Remove before commit */
+	{
+		{"skipscan_iprefix_enabled", PGC_SUSET, DEVELOPER_OPTIONS,
+			NULL, NULL,
+			GUC_NOT_IN_SAMPLE
+		},
+		&skipscan_iprefix_enabled,
+		true,
+		NULL, NULL, NULL
+	},
+
 	{
 		{"integer_datetimes", PGC_INTERNAL, PRESET_OPTIONS,
 			gettext_noop("Shows whether datetimes are integer based."),
@@ -3724,6 +3747,17 @@ struct config_int ConfigureNamesInt[] =
 		NULL, NULL, NULL
 	},
 
+	/* XXX Remove before commit */
+	{
+		{"skipscan_prefix_cols", PGC_SUSET, DEVELOPER_OPTIONS,
+			NULL, NULL,
+			GUC_NOT_IN_SAMPLE
+		},
+		&skipscan_prefix_cols,
+		INDEX_MAX_KEYS, 0, INDEX_MAX_KEYS,
+		NULL, NULL, NULL
+	},
+
 	{
 		/* Can't be set in postgresql.conf */
 		{"server_version_num", PGC_INTERNAL, PRESET_OPTIONS,
-- 
2.49.0