From 7b2ff9a70f739806d9347c4f1bb5d03301060b6b Mon Sep 17 00:00:00 2001 From: Maxime Schoemans 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)