[PATCH] Check that index can return in get_actual_variable_range()

Started by Maxime Schoemans4 months ago9 messages
#1Maxime Schoemans
maxime.schoemans@enterprisedb.com
1 attachment(s)

Hi all,

Some recent changes were made to remove the explicit dependency on btree indexes in some parts of the code. One of these changes was made in commit 9ef1851685b, which allows non-btree indexes to be used in get_actual_variable_range(). A follow-up commit ee1ae8b99f9 fixes the cases where an index doesn’t have a sortopfamily as this is a prerequisite to be used in get_actual_variable_range(). However, I found out recently that indices that have ‘amcanorder = true’ but do not allow index-only-scans (amcanreturn returns false or is NULL) will pass all of the conditions, while they should be rejected since get_actual_variable_range() uses the index-only-scan machinery in get_actual_variable_endpoint().

Attached is a small patch that adds a check in get_actual_variable_range() to reject indices that do not allow index-only scans.

Regards,
Maxime Schoemans

Attachments:

v1-0001-Check-that-index-can-return-in-get_actual_variabl.patchapplication/octet-stream; name=v1-0001-Check-that-index-can-return-in-get_actual_variabl.patch; x-unix-mode=0644Download
From 647d5315ad0976d6eca6df8c433cb4237c8898b0 Mon Sep 17 00:00:00 2001
From: Maxime Schoemans <maxime.schoemans@enterprisedb.com>
Date: Tue, 16 Sep 2025 18:19:58 +0200
Subject: [PATCH v1] Check that index can return in get_actual_variable_range()

Follow-up to 9ef1851685b and ee1ae8b99f9. The min and/or max variables
are retrieved from the index using the index-only-scan machinery, so
first check that we can use it by looking at index->canreturn.
---
 src/backend/utils/adt/selfuncs.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index a96b1b9c0bc..216c034825e 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -6514,6 +6514,13 @@ get_actual_variable_range(PlannerInfo *root, VariableStatData *vardata,
 		if (index->hypothetical)
 			continue;
 
+		/*
+		 * get_actual_variable_endpoint uses the index-only-scan machinery,
+		 * so ignore indexes that can't use it on their first column.
+		 */
+		if (!index->canreturn[0])
+			continue;
+
 		/*
 		 * The first index column must match the desired variable, sortop, and
 		 * collation --- but we can use a descending-order index.
-- 
2.39.5 (Apple Git-154)

#2Aleksander Alekseev
aleksander@tigerdata.com
In reply to: Maxime Schoemans (#1)
Re: [PATCH] Check that index can return in get_actual_variable_range()

Hi Maxime,

Some recent changes were made to remove the explicit dependency on btree indexes in some parts of the code. One of these changes was made in commit 9ef1851685b, which allows non-btree indexes to be used in get_actual_variable_range(). A follow-up commit ee1ae8b99f9 fixes the cases where an index doesn’t have a sortopfamily as this is a prerequisite to be used in get_actual_variable_range(). However, I found out recently that indices that have ‘amcanorder = true’ but do not allow index-only-scans (amcanreturn returns false or is NULL) will pass all of the conditions, while they should be rejected since get_actual_variable_range() uses the index-only-scan machinery in get_actual_variable_endpoint().

Attached is a small patch that adds a check in get_actual_variable_range() to reject indices that do not allow index-only scans.

Thanks for the patch.

Can you think of any test cases we can add to the code base?

--
Best regards,
Aleksander Alekseev

#3Mark Dilger
mark.dilger@enterprisedb.com
In reply to: Aleksander Alekseev (#2)
Re: [PATCH] Check that index can return in get_actual_variable_range()

Testing with the src/test/modules/treeb work in the patch series at [1]/messages/by-id/5083d9ed-837d-47cd-9d40-fded88b62c10@eisentraut.org,
modifying treebcanreturn() to always return false and modifying
_treeb_first(), _treeb_next(), and _treeb_endpoint() to set scan->xs_itup
to NULL rather than to a tuple, without the patch there are a number of
test failures with "ERROR: no data returned for index-only scan", but with
the patch applied those errors still appear. They are raised by
nodeIndexonlyscan.c at line 213, inside IndexOnlyNext(), suggesting that
changing treebcanreturn() to return false was not enough to dissuade the
planner from choosing the index for an index only scan.

Is there a bug in how the system selects an index for an index-only scan?
Or is the combination amcanorder=true && amcanreturn=NULL/false not a valid
choice for an index? If the latter, shouldn't there be a check for that
somewhere, or at least an Assert()?

[1]: /messages/by-id/5083d9ed-837d-47cd-9d40-fded88b62c10@eisentraut.org
/messages/by-id/5083d9ed-837d-47cd-9d40-fded88b62c10@eisentraut.org

On Thu, Sep 18, 2025 at 5:32 AM Aleksander Alekseev <
aleksander@tigerdata.com> wrote:

Hi Maxime,

Some recent changes were made to remove the explicit dependency on btree

indexes in some parts of the code. One of these changes was made in commit
9ef1851685b, which allows non-btree indexes to be used in
get_actual_variable_range(). A follow-up commit ee1ae8b99f9 fixes the cases
where an index doesn’t have a sortopfamily as this is a prerequisite to be
used in get_actual_variable_range(). However, I found out recently that
indices that have ‘amcanorder = true’ but do not allow index-only-scans
(amcanreturn returns false or is NULL) will pass all of the conditions,
while they should be rejected since get_actual_variable_range() uses the
index-only-scan machinery in get_actual_variable_endpoint().

Attached is a small patch that adds a check in

get_actual_variable_range() to reject indices that do not allow index-only
scans.

Thanks for the patch.

Can you think of any test cases we can add to the code base?

--
Best regards,
Aleksander Alekseev

--

*Mark Dilger*

#4Maxime Schoemans
maxime.schoemans@enterprisedb.com
In reply to: Aleksander Alekseev (#2)
Re: [PATCH] Check that index can return in get_actual_variable_range()

On 18 Sep 2025, at 14:31, Aleksander Alekseev <aleksander@tigerdata.com> wrote:

Can you think of any test cases we can add to the code base?

The only idea I have would be adding a new index module in src/test/modules that has this particular property and have a test that will hit get_actual_variable_range().
For example, it could be a new access method that is a copy of btree, but whose amcanreturn property is null and that specifically sets scan->xs_itup to null in amgettuple.

Without the patch, such a test would fail with 'ERROR: no data returned for index-only scan’, while with the patch it would pass.

Best,
Maxime Schoemans

#5Maxime Schoemans
maxime.schoemans@enterprisedb.com
In reply to: Mark Dilger (#3)
Re: [PATCH] Check that index can return in get_actual_variable_range()

On 18 Sep 2025, at 15:36, Mark Dilger <mark.dilger@enterprisedb.com> wrote:

Testing with the src/test/modules/treeb work in the patch series at [1], modifying treebcanreturn() to always return false and modifying _treeb_first(), _treeb_next(), and _treeb_endpoint() to set scan->xs_itup to NULL rather than to a tuple, without the patch there are a number of test failures with "ERROR: no data returned for index-only scan", but with the patch applied those errors still appear. They are raised by nodeIndexonlyscan.c at line 213, inside IndexOnlyNext(), suggesting that changing treebcanreturn() to return false was not enough to dissuade the planner from choosing the index for an index only scan.

Is there a bug in how the system selects an index for an index-only scan? Or is the combination amcanorder=true && amcanreturn=NULL/false not a valid choice for an index? If the latter, shouldn't there be a check for that somewhere, or at least an Assert()?

After a short offline discussion with Mark, we determined that this issue was due to treebproperty still returning true for the AMPROP_RETURNABLE property. This was a small oversight in the testing, but does not indicate a bug in postgres.

#6Aleksander Alekseev
aleksander@tigerdata.com
In reply to: Maxime Schoemans (#4)
Re: [PATCH] Check that index can return in get_actual_variable_range()

Hi Maxime,

Can you think of any test cases we can add to the code base?

The only idea I have would be adding a new index module in src/test/modules that has this particular property and have a test that will hit get_actual_variable_range().
For example, it could be a new access method that is a copy of btree, but whose amcanreturn property is null and that specifically sets scan->xs_itup to null in amgettuple.

Without the patch, such a test would fail with 'ERROR: no data returned for index-only scan’, while with the patch it would pass.

Yes, this is how we typically test cases like this. IMO adding a test
module would be helpful. It can be reused for other scenarios.

--
Best regards,
Aleksander Alekseev

#7Maxime Schoemans
maxime.schoemans@enterprisedb.com
In reply to: Aleksander Alekseev (#6)
2 attachment(s)
Re: [PATCH] Check that index can return in get_actual_variable_range()

On 19 Sep 2025, at 10:20, Aleksander Alekseev <aleksander@tigerdata.com>
wrote:

Yes, this is how we typically test cases like this. IMO adding a test
module would be helpful. It can be reused for other scenarios.

Here is an updated patch set.
- 0001 is unchanged.
- 0002 contains the module that tests the correct behavior of
get_actual_variable_range for non-returning ordering indices.
It contains a copy of the btree handler function with its index-only
capabilities removed. If you apply patch 0002 on master without 0001,
you will see that the test returns an error (ERROR: no data returned
for index-only scan) as it tries to use the index in
get_actual_variable_range, which shouldn’t be the case.

Best,
Maxime Schoemans

Attachments:

v2-0001-Check-that-index-can-return-in-get_actual_variabl.patchapplication/octet-stream; name=v2-0001-Check-that-index-can-return-in-get_actual_variabl.patch; x-unix-mode=0644Download
From 5efc08f9a643fbb713bacd53b462aaafd63b8c1f Mon Sep 17 00:00:00 2001
From: Maxime Schoemans <maxime.schoemans@enterprisedb.com>
Date: Tue, 16 Sep 2025 18:19:58 +0200
Subject: [PATCH v2 1/2] Check that index can return in
 get_actual_variable_range()

Follow-up to 9ef1851685b and ee1ae8b99f9. The min and/or max variables
are retrieved from the index using the index-only-scan machinery, so
first check that we can use it by looking at index->canreturn.
---
 src/backend/utils/adt/selfuncs.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index e5e066a5537..eff4372aa90 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -6574,6 +6574,13 @@ get_actual_variable_range(PlannerInfo *root, VariableStatData *vardata,
 		if (index->hypothetical)
 			continue;
 
+		/*
+		 * get_actual_variable_endpoint uses the index-only-scan machinery,
+		 * so ignore indexes that can't use it on their first column.
+		 */
+		if (!index->canreturn[0])
+			continue;
+
 		/*
 		 * The first index column must match the desired variable, sortop, and
 		 * collation --- but we can use a descending-order index.
-- 
2.39.5 (Apple Git-154)

v2-0002-Add-btree_noreturn-module-as-example-of-a-non-ret.patchapplication/octet-stream; name=v2-0002-Add-btree_noreturn-module-as-example-of-a-non-ret.patch; x-unix-mode=0644Download
From 7b2ff9a70f739806d9347c4f1bb5d03301060b6b Mon Sep 17 00:00:00 2001
From: Maxime Schoemans <maxime.schoemans@enterprisedb.com>
Date: Mon, 22 Sep 2025 16:21:24 +0200
Subject: [PATCH v2 2/2] Add btree_noreturn module as example of a
 non-returning, ordering index

Until now, each code or contrib index am that has amcanorder = true,
also has amcanreturn returning true. This causes some code to make the
assumption that this is always the case, as noted and fixed in the
previous commit. Such code would thus fail on extension indices that did
not match this assumption.

This commit adds a test module that copies the btree index am but sets
its amcanreturn to NULL, to effectively disable support for index-only
scans. This allows us to check that the behavior of such ordering but
non-returning indices is correctly handled. Currently, this module just
checks that get_actual_variable_range, which uses the index-only-scan
machinery, correctly discards the btree_noreturn index.
---
 src/test/modules/Makefile                     |   1 +
 src/test/modules/btree_noreturn/.gitignore    |   3 +
 src/test/modules/btree_noreturn/Makefile      |  20 ++++
 .../btree_noreturn/btree_noreturn--1.0.sql    |  26 +++++
 .../modules/btree_noreturn/btree_noreturn.c   | 106 ++++++++++++++++++
 .../btree_noreturn/btree_noreturn.control     |   5 +
 .../expected/btree_noreturn.out               |  19 ++++
 src/test/modules/btree_noreturn/meson.build   |  33 ++++++
 .../btree_noreturn/sql/btree_noreturn.sql     |  18 +++
 src/test/modules/meson.build                  |   1 +
 10 files changed, 232 insertions(+)
 create mode 100644 src/test/modules/btree_noreturn/.gitignore
 create mode 100644 src/test/modules/btree_noreturn/Makefile
 create mode 100644 src/test/modules/btree_noreturn/btree_noreturn--1.0.sql
 create mode 100644 src/test/modules/btree_noreturn/btree_noreturn.c
 create mode 100644 src/test/modules/btree_noreturn/btree_noreturn.control
 create mode 100644 src/test/modules/btree_noreturn/expected/btree_noreturn.out
 create mode 100644 src/test/modules/btree_noreturn/meson.build
 create mode 100644 src/test/modules/btree_noreturn/sql/btree_noreturn.sql

diff --git a/src/test/modules/Makefile b/src/test/modules/Makefile
index 902a7954101..a6d98a4d4fb 100644
--- a/src/test/modules/Makefile
+++ b/src/test/modules/Makefile
@@ -6,6 +6,7 @@ include $(top_builddir)/src/Makefile.global
 
 SUBDIRS = \
 		  brin \
+		  btree_noreturn \
 		  commit_ts \
 		  delay_execution \
 		  dummy_index_am \
diff --git a/src/test/modules/btree_noreturn/.gitignore b/src/test/modules/btree_noreturn/.gitignore
new file mode 100644
index 00000000000..44d119cfcc2
--- /dev/null
+++ b/src/test/modules/btree_noreturn/.gitignore
@@ -0,0 +1,3 @@
+# Generated subdirectories
+/log/
+/results/
diff --git a/src/test/modules/btree_noreturn/Makefile b/src/test/modules/btree_noreturn/Makefile
new file mode 100644
index 00000000000..7b3695aaa3d
--- /dev/null
+++ b/src/test/modules/btree_noreturn/Makefile
@@ -0,0 +1,20 @@
+# src/test/modules/btree_noreturn/Makefile
+
+MODULES = btree_noreturn
+
+EXTENSION = btree_noreturn
+DATA = btree_noreturn--1.0.sql
+PGFILEDESC = "btree_noreturn - btree copy without support for index-only scans"
+
+REGRESS = btree_noreturn
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = src/test/modules/btree_noreturn
+top_builddir = ../../../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
diff --git a/src/test/modules/btree_noreturn/btree_noreturn--1.0.sql b/src/test/modules/btree_noreturn/btree_noreturn--1.0.sql
new file mode 100644
index 00000000000..0e0ea790f66
--- /dev/null
+++ b/src/test/modules/btree_noreturn/btree_noreturn--1.0.sql
@@ -0,0 +1,26 @@
+/* src/test/modules/btree_noreturn/btree_noreturn--1.0.sql */
+
+-- complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "CREATE EXTENSION btree_noreturn" to load this file. \quit
+
+CREATE FUNCTION btnrhandler(internal)
+RETURNS index_am_handler
+AS 'MODULE_PATHNAME'
+LANGUAGE C;
+
+-- Access method
+CREATE ACCESS METHOD btree_noreturn TYPE INDEX HANDLER btnrhandler;
+COMMENT ON ACCESS METHOD btree_noreturn IS 'btree copy without support for index-only scans';
+
+-- Operator classes
+CREATE OPERATOR CLASS int4_ops
+  DEFAULT FOR TYPE integer USING btree_noreturn AS
+  OPERATOR 1 <  (integer,integer),
+  OPERATOR 2 <= (integer,integer),
+  OPERATOR 3 =  (integer,integer),
+  OPERATOR 4 >= (integer,integer),
+  OPERATOR 5 >  (integer,integer),
+  FUNCTION 1 btint4cmp(integer, integer),
+  FUNCTION 2 btint4sortsupport(internal),
+  FUNCTION 3 in_range(integer,integer,integer,bool,bool),
+  FUNCTION 4 btequalimage(oid);
diff --git a/src/test/modules/btree_noreturn/btree_noreturn.c b/src/test/modules/btree_noreturn/btree_noreturn.c
new file mode 100644
index 00000000000..7e26c960786
--- /dev/null
+++ b/src/test/modules/btree_noreturn/btree_noreturn.c
@@ -0,0 +1,106 @@
+/*-------------------------------------------------------------------------
+ *
+ * btree_noreturn.c
+ *	  Copy of the B-Tree index am, without support for index-only scans.
+ *
+ * Copyright (c) 2025, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ *	  src/test/modules/btree_noreturn/btree_noreturn.c
+ *
+ *-------------------------------------------------------------------------
+ */
+#include "postgres.h"
+#include "access/relscan.h"
+#include "access/nbtree.h"
+#include "commands/vacuum.h"
+#include "utils/index_selfuncs.h"
+
+PG_MODULE_MAGIC;
+
+static bool btnrgettuple(IndexScanDesc scan, ScanDirection dir);
+
+/*
+ * btree_noreturn handler function
+ *
+ * Copy of btree handler with three modifications to
+ * remove support for index-only-scans:
+ *  1. amcanreturn - set to NULL
+ *  2. amproperty - set to NULL
+ *  3. amgettuple - call btgettuple and set xs_itup to NULL
+ */
+PG_FUNCTION_INFO_V1(btnrhandler);
+Datum
+btnrhandler(PG_FUNCTION_ARGS)
+{
+	IndexAmRoutine *amroutine = makeNode(IndexAmRoutine);
+
+	amroutine->amstrategies = BTMaxStrategyNumber;
+	amroutine->amsupport = BTNProcs;
+	amroutine->amoptsprocnum = BTOPTIONS_PROC;
+	amroutine->amcanorder = true;
+	amroutine->amcanorderbyop = false;
+	amroutine->amcanbackward = true;
+	amroutine->amcanunique = true;
+	amroutine->amcanmulticol = true;
+	amroutine->amoptionalkey = true;
+	amroutine->amsearcharray = true;
+	amroutine->amsearchnulls = true;
+	amroutine->amstorage = false;
+	amroutine->amclusterable = true;
+	amroutine->ampredlocks = true;
+	amroutine->amcanparallel = true;
+	amroutine->amtranslatestrategy = bttranslatestrategy;
+	amroutine->amtranslatecmptype = bttranslatecmptype;
+	amroutine->amcanbuildparallel = true;
+	amroutine->amcaninclude = true;
+	amroutine->amusemaintenanceworkmem = false;
+	amroutine->amsummarizing = false;
+	amroutine->amparallelvacuumoptions =
+		VACUUM_OPTION_PARALLEL_BULKDEL | VACUUM_OPTION_PARALLEL_COND_CLEANUP;
+	amroutine->amkeytype = InvalidOid;
+
+	amroutine->ambuild = btbuild;
+	amroutine->ambuildempty = btbuildempty;
+	amroutine->aminsert = btinsert;
+	amroutine->aminsertcleanup = NULL;
+	amroutine->ambulkdelete = btbulkdelete;
+	amroutine->amvacuumcleanup = btvacuumcleanup;
+	amroutine->amcanreturn = NULL;
+	amroutine->amcostestimate = btcostestimate;
+	amroutine->amgettreeheight = btgettreeheight;
+	amroutine->amoptions = btoptions;
+	amroutine->amproperty = NULL;
+	amroutine->ambuildphasename = btbuildphasename;
+	amroutine->amvalidate = btvalidate;
+	amroutine->amadjustmembers = btadjustmembers;
+	amroutine->ambeginscan = btbeginscan;
+	amroutine->amrescan = btrescan;
+	amroutine->amgettuple = btnrgettuple;
+	amroutine->amgetbitmap = btgetbitmap;
+	amroutine->amendscan = btendscan;
+	amroutine->ammarkpos = btmarkpos;
+	amroutine->amrestrpos = btrestrpos;
+	amroutine->amestimateparallelscan = btestimateparallelscan;
+	amroutine->aminitparallelscan = btinitparallelscan;
+	amroutine->amparallelrescan = btparallelrescan;
+
+	PG_RETURN_POINTER(amroutine);
+}
+
+/*
+ *	btnrgettuple() -- Get the next tuple in the scan.
+ */
+static bool
+btnrgettuple(IndexScanDesc scan, ScanDirection dir)
+{
+	bool res = btgettuple(scan, dir);
+
+	/*
+	 * btgettuple sets xs_itup, so set it back to to NULL
+	 * to simulate an index that does not return data.
+	 */
+	scan->xs_itup = NULL;
+
+	return res;
+}
diff --git a/src/test/modules/btree_noreturn/btree_noreturn.control b/src/test/modules/btree_noreturn/btree_noreturn.control
new file mode 100644
index 00000000000..886446b7df8
--- /dev/null
+++ b/src/test/modules/btree_noreturn/btree_noreturn.control
@@ -0,0 +1,5 @@
+# btree_noreturn extension
+comment = 'btree_noreturn - btree copy without support for index-only scans'
+default_version = '1.0'
+module_pathname = '$libdir/btree_noreturn'
+relocatable = true
diff --git a/src/test/modules/btree_noreturn/expected/btree_noreturn.out b/src/test/modules/btree_noreturn/expected/btree_noreturn.out
new file mode 100644
index 00000000000..eb1b8f3002e
--- /dev/null
+++ b/src/test/modules/btree_noreturn/expected/btree_noreturn.out
@@ -0,0 +1,19 @@
+CREATE EXTENSION btree_noreturn;
+-- Load a table with values between 1 and 1000
+CREATE TABLE btnr_test_tab (i) AS
+  SELECT i FROM generate_series(1, 1000) i;
+-- Build the stats histogram
+ANALYZE btnr_test_tab;
+-- Create a btree_noreturn index on the data
+CREATE INDEX btnr_test_idx ON btnr_test_tab USING btree_noreturn (i);
+--
+-- Make sure that get_actual_variable_range correctly discards
+-- the btree_noreturn index instead of using it to compute the max
+-- in ineq_histogram_selectivity.
+--
+SELECT count(i) FROM btnr_test_tab WHERE i > 1001;
+ count 
+-------
+     0
+(1 row)
+
diff --git a/src/test/modules/btree_noreturn/meson.build b/src/test/modules/btree_noreturn/meson.build
new file mode 100644
index 00000000000..146240d68a3
--- /dev/null
+++ b/src/test/modules/btree_noreturn/meson.build
@@ -0,0 +1,33 @@
+# Copyright (c) 2022-2025, PostgreSQL Global Development Group
+
+btree_noreturn_sources = files(
+  'btree_noreturn.c',
+)
+
+if host_system == 'windows'
+  btree_noreturn_sources += rc_lib_gen.process(win32ver_rc, extra_args: [
+    '--NAME', 'btree_noreturn',
+    '--FILEDESC', 'btree_noreturn - btree copy without support for index-only scans',])
+endif
+
+btree_noreturn = shared_module('btree_noreturn',
+  btree_noreturn_sources,
+  kwargs: pg_test_mod_args,
+)
+test_install_libs += btree_noreturn
+
+test_install_data += files(
+  'btree_noreturn.control',
+  'btree_noreturn--1.0.sql',
+)
+
+tests += {
+  'name': 'btree_noreturn',
+  'sd': meson.current_source_dir(),
+  'bd': meson.current_build_dir(),
+  'regress': {
+    'sql': [
+      'btree_noreturn',
+    ],
+  },
+}
diff --git a/src/test/modules/btree_noreturn/sql/btree_noreturn.sql b/src/test/modules/btree_noreturn/sql/btree_noreturn.sql
new file mode 100644
index 00000000000..fa919710edf
--- /dev/null
+++ b/src/test/modules/btree_noreturn/sql/btree_noreturn.sql
@@ -0,0 +1,18 @@
+CREATE EXTENSION btree_noreturn;
+
+-- Load a table with values between 1 and 1000
+CREATE TABLE btnr_test_tab (i) AS
+  SELECT i FROM generate_series(1, 1000) i;
+
+-- Build the stats histogram
+ANALYZE btnr_test_tab;
+
+-- Create a btree_noreturn index on the data
+CREATE INDEX btnr_test_idx ON btnr_test_tab USING btree_noreturn (i);
+
+--
+-- Make sure that get_actual_variable_range correctly discards
+-- the btree_noreturn index instead of using it to compute the max
+-- in ineq_histogram_selectivity.
+--
+SELECT count(i) FROM btnr_test_tab WHERE i > 1001;
diff --git a/src/test/modules/meson.build b/src/test/modules/meson.build
index 14fc761c4cf..40a7b656321 100644
--- a/src/test/modules/meson.build
+++ b/src/test/modules/meson.build
@@ -1,6 +1,7 @@
 # Copyright (c) 2022-2025, PostgreSQL Global Development Group
 
 subdir('brin')
+subdir('btree_noreturn')
 subdir('commit_ts')
 subdir('delay_execution')
 subdir('dummy_index_am')
-- 
2.39.5 (Apple Git-154)

#8Peter Eisentraut
peter@eisentraut.org
In reply to: Maxime Schoemans (#7)
Re: [PATCH] Check that index can return in get_actual_variable_range()

On 22.09.25 16:38, Maxime Schoemans wrote:

On 19 Sep 2025, at 10:20, Aleksander Alekseev <aleksander@tigerdata.com>
wrote:

Yes, this is how we typically test cases like this. IMO adding a test
module would be helpful. It can be reused for other scenarios.

Here is an updated patch set.
- 0001 is unchanged.
- 0002 contains the module that tests the correct behavior of
get_actual_variable_range for non-returning ordering indices.
It contains a copy of the btree handler function with its index-only
capabilities removed. If you apply patch 0002 on master without 0001,
you will see that the test returns an error (ERROR: no data returned
for index-only scan) as it tries to use the index in
get_actual_variable_range, which shouldn’t be the case.

I have committed the first patch.

The test suite is probably a bit too bulky for testing this particular
niche behavior. Also, it doesn't work with assertions enabled because
of the hardcoded BTREE_AM_OID in src/include/access/nbtree.h. So I
don't plan to commit that. But it's good to have it in the archive, and
perhaps it can be part of a larger test suite for the index AM API at
some point in the future.

#9Maxime Schoemans
maxime.schoemans@enterprisedb.com
In reply to: Peter Eisentraut (#8)
Re: [PATCH] Check that index can return in get_actual_variable_range()

On 28 Oct 2025, at 10:20, Peter Eisentraut <peter@eisentraut.org> wrote:

I have committed the first patch.

Great, thank you for looking at this.

The test suite is probably a bit too bulky for testing this particular niche behavior. Also, it doesn't work with assertions enabled because of the hardcoded BTREE_AM_OID in src/include/access/nbtree.h. So I don't plan to commit that. But it's good to have it in the archive, and perhaps it can be part of a larger test suite for the index AM API at some point in the future.

And I agree, the test suite is probably overkill for this two-line patch, but it might be good to have around for later.