]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
Test pg_plan_advice using a new test_plan_advice module.
authorRobert Haas <rhaas@postgresql.org>
Tue, 17 Mar 2026 18:00:45 +0000 (14:00 -0400)
committerRobert Haas <rhaas@postgresql.org>
Tue, 17 Mar 2026 18:06:26 +0000 (14:06 -0400)
The TAP test included in this new module runs the regression tests
with pg_plan_advice loaded. It arranges for each query to be planned
twice.  The first time, we generate plan advice. The second time, we
replan the query using the resulting advice string. If the tests
fail, that means that using pg_plan_advice to tell the planner to
do what it was going to do anyway breaks something, which indicates
a problem either with pg_plan_advice or with the planner.

The test also enables pg_plan_advice.feedback_warnings, so that if the
plan advice fails to apply cleanly when the query is replanned, a
failure will occur.

Reviewed-by: Alexandra Wang <alexandra.wang.oss@gmail.com>
Reviewed-by: Lukas Fittl <lukas@fittl.com>
Discussion: http://postgr.es/m/CA%2BTgmoZzM2i%2Bp-Rxdphs4qx7sshn-kzxF91ASQ5duOo0dFRXLQ%40mail.gmail.com

src/test/modules/Makefile
src/test/modules/meson.build
src/test/modules/test_plan_advice/Makefile [new file with mode: 0644]
src/test/modules/test_plan_advice/meson.build [new file with mode: 0644]
src/test/modules/test_plan_advice/t/001_replan_regress.pl [new file with mode: 0644]
src/test/modules/test_plan_advice/test_plan_advice.c [new file with mode: 0644]

index 4ac5c84db439b59fb6bfcacf40d1389dc3eba936..a1540269cf5b4a8d20a38cfc5885f9eb6a54df92 100644 (file)
@@ -39,6 +39,7 @@ SUBDIRS = \
                  test_oat_hooks \
                  test_parser \
                  test_pg_dump \
+                 test_plan_advice \
                  test_predtest \
                  test_radixtree \
                  test_rbtree \
index e2b3eef4136858cda67d34101c2b9f3daddaca0d..7c052803c983082e228c447393450d394ba3260b 100644 (file)
@@ -40,6 +40,7 @@ subdir('test_misc')
 subdir('test_oat_hooks')
 subdir('test_parser')
 subdir('test_pg_dump')
+subdir('test_plan_advice')
 subdir('test_predtest')
 subdir('test_radixtree')
 subdir('test_rbtree')
diff --git a/src/test/modules/test_plan_advice/Makefile b/src/test/modules/test_plan_advice/Makefile
new file mode 100644 (file)
index 0000000..d101fd6
--- /dev/null
@@ -0,0 +1,28 @@
+# src/test/modules/test_plan_advice/Makefile
+
+PGFILEDESC = "test_plan_advice - test whether generated plan advice works"
+
+MODULE_big = test_plan_advice
+OBJS = \
+       $(WIN32RES) \
+       test_plan_advice.o
+
+EXTRA_INSTALL = contrib/pg_plan_advice
+
+TAP_TESTS = 1
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+override CPPFLAGS += -I$(includedir_server)/contrib/pg_plan_advice
+else
+subdir = src/test/modules/test_plan_advice
+top_builddir = ../../../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+override CPPFLAGS += -I$(top_srcdir)/contrib/pg_plan_advice
+endif
+
+REGRESS_SHLIB=$(abs_top_builddir)/src/test/regress/regress$(DLSUFFIX)
+export REGRESS_SHLIB
diff --git a/src/test/modules/test_plan_advice/meson.build b/src/test/modules/test_plan_advice/meson.build
new file mode 100644 (file)
index 0000000..afde420
--- /dev/null
@@ -0,0 +1,29 @@
+# Copyright (c) 2022-2026, PostgreSQL Global Development Group
+
+test_plan_advice_sources = files(
+  'test_plan_advice.c',
+)
+
+if host_system == 'windows'
+  test_plan_advice_sources += rc_lib_gen.process(win32ver_rc, extra_args: [
+    '--NAME', 'test_plan_advice',
+    '--FILEDESC', 'test_plan_advice - test whether generated plan advice works',])
+endif
+
+test_plan_advice = shared_module('test_plan_advice',
+  test_plan_advice_sources,
+  include_directories: pg_plan_advice_inc,
+  kwargs: pg_test_mod_args,
+)
+test_install_libs += test_plan_advice
+
+tests += {
+  'name': 'test_plan_advice',
+  'sd': meson.current_source_dir(),
+  'bd': meson.current_build_dir(),
+  'tap': {
+    'tests': [
+      't/001_replan_regress.pl',
+    ],
+  },
+}
diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl
new file mode 100644 (file)
index 0000000..38ffa4d
--- /dev/null
@@ -0,0 +1,65 @@
+# Copyright (c) 2021-2026, PostgreSQL Global Development Group
+
+# Run the core regression tests under pg_plan_advice to check for problems.
+use strict;
+use warnings FATAL => 'all';
+
+use Cwd            qw(abs_path);
+use File::Basename qw(dirname);
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+# Initialize the primary node
+my $node = PostgreSQL::Test::Cluster->new('main');
+$node->init();
+
+# Set up our desired configuration.
+$node->append_conf('postgresql.conf', <<EOM);
+shared_preload_libraries='test_plan_advice'
+pg_plan_advice.always_explain_supplied_advice=false
+pg_plan_advice.feedback_warnings=true
+EOM
+$node->start;
+
+my $srcdir = abs_path("../../../..");
+
+# --dlpath is needed to be able to find the location of regress.so
+# and any libraries the regression tests require.
+my $dlpath = dirname($ENV{REGRESS_SHLIB});
+
+# --outputdir points to the path where to place the output files.
+my $outputdir = $PostgreSQL::Test::Utils::tmp_check;
+
+# --inputdir points to the path of the input files.
+my $inputdir = "$srcdir/src/test/regress";
+
+# Run the tests.
+my $rc =
+  system($ENV{PG_REGRESS} . " "
+         . "--bindir= "
+         . "--dlpath=\"$dlpath\" "
+         . "--host=" . $node->host . " "
+         . "--port=" . $node->port . " "
+         . "--schedule=$srcdir/src/test/regress/parallel_schedule "
+         . "--max-concurrent-tests=20 "
+         . "--inputdir=\"$inputdir\" "
+         . "--outputdir=\"$outputdir\"");
+
+# Dump out the regression diffs file, if there is one
+if ($rc != 0)
+{
+       my $diffs = "$outputdir/regression.diffs";
+       if (-e $diffs)
+       {
+               print "=== dumping $diffs ===\n";
+               print slurp_file($diffs);
+               print "=== EOF ===\n";
+       }
+}
+
+# Report results
+is($rc, 0, 'regression tests pass');
+
+done_testing();
diff --git a/src/test/modules/test_plan_advice/test_plan_advice.c b/src/test/modules/test_plan_advice/test_plan_advice.c
new file mode 100644 (file)
index 0000000..cff5039
--- /dev/null
@@ -0,0 +1,143 @@
+/*-------------------------------------------------------------------------
+ *
+ * test_plan_advice.c
+ *       Test pg_plan_advice by planning every query with generated advice.
+ *
+ * With this module loaded, every time a query is executed, we end up
+ * planning it twice. The first time we plan it, we generate plan advice,
+ * which we then feed back to pg_plan_advice as the supplied plan advice.
+ * It is then planned a second time using that advice. This hopefully
+ * allows us to detect cases where the advice is incorrect or causes
+ * failures or plan changes for some reason.
+ *
+ * Copyright (c) 2016-2026, PostgreSQL Global Development Group
+ *
+ *       src/test/modules/test_plan_advice/test_plan_advice.c
+ *
+ *-------------------------------------------------------------------------
+ */
+#include "postgres.h"
+
+#include "access/xact.h"
+#include "fmgr.h"
+#include "optimizer/optimizer.h"
+#include "pg_plan_advice.h"
+#include "utils/guc.h"
+
+PG_MODULE_MAGIC;
+
+static bool in_recursion = false;
+
+static char *test_plan_advice_advisor(PlannerGlobal *glob,
+                                                                         Query *parse,
+                                                                         const char *query_string,
+                                                                         int cursorOptions,
+                                                                         ExplainState *es);
+static DefElem *find_defelem_by_defname(List *deflist, char *defname);
+
+/*
+ * Initialize this module.
+ */
+void
+_PG_init(void)
+{
+       void            (*add_advisor_fn) (pg_plan_advice_advisor_hook hook);
+
+       /*
+        * Ask pg_plan_advice to get advice strings from test_plan_advice_advisor
+        */
+       add_advisor_fn =
+               load_external_function("pg_plan_advice", "pg_plan_advice_add_advisor",
+                                                          true, NULL);
+
+       (*add_advisor_fn) (test_plan_advice_advisor);
+}
+
+/*
+ * Re-plan the given query and return the generated advice string as the
+ * supplied advice.
+ */
+static char *
+test_plan_advice_advisor(PlannerGlobal *glob, Query *parse,
+                                                const char *query_string, int cursorOptions,
+                                                ExplainState *es)
+{
+       PlannedStmt *pstmt;
+       int                     save_nestlevel = 0;
+       DefElem    *pgpa_item;
+       DefElem    *advice_string_item;
+
+       /*
+        * Since this function is called from the planner and triggers planning,
+        * we need a recursion guard.
+        */
+       if (in_recursion)
+               return NULL;
+
+       PG_TRY();
+       {
+               in_recursion = true;
+
+               /*
+                * Planning can trigger expression evaluation, which can result in
+                * sending NOTICE messages or other output to the client. To avoid
+                * that, we set client_min_messages = ERROR in the hopes of getting
+                * the same output with and without this module.
+                *
+                * We also need to set pg_plan_advice.always_store_advice_details so
+                * that pg_plan_advice will generate an advice string, since the whole
+                * point of this function is to get access to that.
+                */
+               save_nestlevel = NewGUCNestLevel();
+               set_config_option("client_min_messages", "error",
+                                                 PGC_SUSET, PGC_S_SESSION,
+                                                 GUC_ACTION_SAVE, true, 0, false);
+               set_config_option("pg_plan_advice.always_store_advice_details", "true",
+                                                 PGC_SUSET, PGC_S_SESSION,
+                                                 GUC_ACTION_SAVE, true, 0, false);
+
+               /*
+                * Replan. We must copy the Query, because the planner modifies it.
+                * (As noted elsewhere, that's unfortunate; perhaps it will be fixed
+                * some day.)
+                */
+               pstmt = planner(copyObject(parse), query_string, cursorOptions,
+                                               glob->boundParams, es);
+       }
+       PG_FINALLY();
+       {
+               in_recursion = false;
+       }
+       PG_END_TRY();
+
+       /* Roll back any GUC changes */
+       if (save_nestlevel > 0)
+               AtEOXact_GUC(false, save_nestlevel);
+
+       /* Extract and return the advice string */
+       pgpa_item = find_defelem_by_defname(pstmt->extension_state,
+                                                                               "pg_plan_advice");
+       if (pgpa_item == NULL)
+               elog(ERROR, "extension state for pg_plan_advice not found");
+       advice_string_item = find_defelem_by_defname((List *) pgpa_item->arg,
+                                                                                                "advice_string");
+       if (advice_string_item == NULL)
+               elog(ERROR,
+                        "advice string for pg_plan_advice not found in extension state");
+       return strVal(advice_string_item->arg);
+}
+
+/*
+ * Search a list of DefElem objects for a given defname.
+ */
+static DefElem *
+find_defelem_by_defname(List *deflist, char *defname)
+{
+       foreach_node(DefElem, item, deflist)
+       {
+               if (strcmp(item->defname, defname) == 0)
+                       return item;
+       }
+
+       return NULL;
+}