On Wed, Aug 25, 2021 at 04:16:08PM +0900, Michael Paquier wrote: > I was just looking at your patch, and I think that you should move all > the past compatibility tests into a separate test file, in a way > consistent to what we do in contrib/pageinspect/ for > oldextversions.sql. I would suggest to use the same file names, while > on it.
The current commit fest is ending, and it would be a waste to do nothing here, so I have looked at what you proposed and reworked it. The patch was blindly testing pg_stat_statements_reset() in all the versions bumped with the same query on pg_stat_statements done each time, which does not help in checking the actual parts of the code that have changed, and there are two of them: - pg_stat_statements_reset() execution got authorized for pg_read_all_stats once in 1.6. - pg_stat_statements() has been extended in 1.8, so we could just have one query stressing this function in the tests for <= 1.7. There is also no need for tests on 1.9, which is the latest version. Tests for this one should be added once we bump the code to the next version. At the end I finish with the attached, counting for the back-and-forth game with pg_read_all_stats. -- Michael
From 9742f2a8b717310ff75ac061f274cbc0502ac8b8 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Thu, 30 Sep 2021 11:10:09 +0900
Subject: [PATCH v4] Add some tests for past versions of pg_stat_statements
---
contrib/pg_stat_statements/Makefile | 2 +-
.../expected/oldextversions.out | 139 ++++++++++++++++++
.../pg_stat_statements/sql/oldextversions.sql | 39 +++++
3 files changed, 179 insertions(+), 1 deletion(-)
create mode 100644 contrib/pg_stat_statements/expected/oldextversions.out
create mode 100644 contrib/pg_stat_statements/sql/oldextversions.sql
diff --git a/contrib/pg_stat_statements/Makefile b/contrib/pg_stat_statements/Makefile
index 3ec627b956..7fabd96f38 100644
--- a/contrib/pg_stat_statements/Makefile
+++ b/contrib/pg_stat_statements/Makefile
@@ -16,7 +16,7 @@ PGFILEDESC = "pg_stat_statements - execution statistics of SQL statements"
LDFLAGS_SL += $(filter -lm, $(LIBS))
REGRESS_OPTS = --temp-config $(top_srcdir)/contrib/pg_stat_statements/pg_stat_statements.conf
-REGRESS = pg_stat_statements
+REGRESS = pg_stat_statements oldextversions
# Disabled because these tests require "shared_preload_libraries=pg_stat_statements",
# which typical installcheck users do not have (e.g. buildfarm clients).
NO_INSTALLCHECK = 1
diff --git a/contrib/pg_stat_statements/expected/oldextversions.out b/contrib/pg_stat_statements/expected/oldextversions.out
new file mode 100644
index 0000000000..515d8fd736
--- /dev/null
+++ b/contrib/pg_stat_statements/expected/oldextversions.out
@@ -0,0 +1,139 @@
+-- test old extension version entry points
+CREATE EXTENSION pg_stat_statements WITH VERSION '1.4';
+-- Execution of pg_stat_statements_reset() is granted to pg_read_all_stats
+-- since 1.5, so this fails.
+SET SESSION AUTHORIZATION pg_read_all_stats;
+SELECT pg_stat_statements_reset();
+ERROR: permission denied for function pg_stat_statements_reset
+RESET SESSION AUTHORIZATION;
+AlTER EXTENSION pg_stat_statements UPDATE TO '1.5';
+-- Execution of pg_stat_statements_reset() should be granted to
+-- pg_read_all_stats now, so this works.
+SET SESSION AUTHORIZATION pg_read_all_stats;
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset
+--------------------------
+
+(1 row)
+
+RESET SESSION AUTHORIZATION;
+-- And in 1.6 it got restricted back to superusers.
+AlTER EXTENSION pg_stat_statements UPDATE TO '1.6';
+SET SESSION AUTHORIZATION pg_read_all_stats;
+SELECT pg_stat_statements_reset();
+ERROR: permission denied for function pg_stat_statements_reset
+RESET SESSION AUTHORIZATION;
+SELECT pg_get_functiondef('pg_stat_statements_reset'::regproc);
+ pg_get_functiondef
+-------------------------------------------------------------------------------
+ CREATE OR REPLACE FUNCTION public.pg_stat_statements_reset() +
+ RETURNS void +
+ LANGUAGE c +
+ PARALLEL SAFE +
+ AS '$libdir/pg_stat_statements', $function$pg_stat_statements_reset$function$+
+
+(1 row)
+
+-- New function for pg_stat_statements_reset introduced, still
+-- restricted for non-superusers.
+AlTER EXTENSION pg_stat_statements UPDATE TO '1.7';
+SET SESSION AUTHORIZATION pg_read_all_stats;
+SELECT pg_stat_statements_reset();
+ERROR: permission denied for function pg_stat_statements_reset
+RESET SESSION AUTHORIZATION;
+SELECT pg_get_functiondef('pg_stat_statements_reset'::regproc);
+ pg_get_functiondef
+--------------------------------------------------------------------------------------------------------------------------------
+ CREATE OR REPLACE FUNCTION public.pg_stat_statements_reset(userid oid DEFAULT 0, dbid oid DEFAULT 0, queryid bigint DEFAULT 0)+
+ RETURNS void +
+ LANGUAGE c +
+ PARALLEL SAFE STRICT +
+ AS '$libdir/pg_stat_statements', $function$pg_stat_statements_reset_1_7$function$ +
+
+(1 row)
+
+\d pg_stat_statements
+ View "public.pg_stat_statements"
+ Column | Type | Collation | Nullable | Default
+---------------------+------------------+-----------+----------+---------
+ userid | oid | | |
+ dbid | oid | | |
+ queryid | bigint | | |
+ query | text | | |
+ calls | bigint | | |
+ total_time | double precision | | |
+ min_time | double precision | | |
+ max_time | double precision | | |
+ mean_time | double precision | | |
+ stddev_time | double precision | | |
+ rows | bigint | | |
+ shared_blks_hit | bigint | | |
+ shared_blks_read | bigint | | |
+ shared_blks_dirtied | bigint | | |
+ shared_blks_written | bigint | | |
+ local_blks_hit | bigint | | |
+ local_blks_read | bigint | | |
+ local_blks_dirtied | bigint | | |
+ local_blks_written | bigint | | |
+ temp_blks_read | bigint | | |
+ temp_blks_written | bigint | | |
+ blk_read_time | double precision | | |
+ blk_write_time | double precision | | |
+
+SELECT count(*) > 0 AS has_data FROM pg_stat_statements;
+ has_data
+----------
+ t
+(1 row)
+
+-- New functions and views for pg_stat_statements in 1.8
+AlTER EXTENSION pg_stat_statements UPDATE TO '1.8';
+\d pg_stat_statements
+ View "public.pg_stat_statements"
+ Column | Type | Collation | Nullable | Default
+---------------------+------------------+-----------+----------+---------
+ userid | oid | | |
+ dbid | oid | | |
+ queryid | bigint | | |
+ query | text | | |
+ plans | bigint | | |
+ total_plan_time | double precision | | |
+ min_plan_time | double precision | | |
+ max_plan_time | double precision | | |
+ mean_plan_time | double precision | | |
+ stddev_plan_time | double precision | | |
+ calls | bigint | | |
+ total_exec_time | double precision | | |
+ min_exec_time | double precision | | |
+ max_exec_time | double precision | | |
+ mean_exec_time | double precision | | |
+ stddev_exec_time | double precision | | |
+ rows | bigint | | |
+ shared_blks_hit | bigint | | |
+ shared_blks_read | bigint | | |
+ shared_blks_dirtied | bigint | | |
+ shared_blks_written | bigint | | |
+ local_blks_hit | bigint | | |
+ local_blks_read | bigint | | |
+ local_blks_dirtied | bigint | | |
+ local_blks_written | bigint | | |
+ temp_blks_read | bigint | | |
+ temp_blks_written | bigint | | |
+ blk_read_time | double precision | | |
+ blk_write_time | double precision | | |
+ wal_records | bigint | | |
+ wal_fpi | bigint | | |
+ wal_bytes | numeric | | |
+
+SELECT pg_get_functiondef('pg_stat_statements_reset'::regproc);
+ pg_get_functiondef
+--------------------------------------------------------------------------------------------------------------------------------
+ CREATE OR REPLACE FUNCTION public.pg_stat_statements_reset(userid oid DEFAULT 0, dbid oid DEFAULT 0, queryid bigint DEFAULT 0)+
+ RETURNS void +
+ LANGUAGE c +
+ PARALLEL SAFE STRICT +
+ AS '$libdir/pg_stat_statements', $function$pg_stat_statements_reset_1_7$function$ +
+
+(1 row)
+
+DROP EXTENSION pg_stat_statements;
diff --git a/contrib/pg_stat_statements/sql/oldextversions.sql b/contrib/pg_stat_statements/sql/oldextversions.sql
new file mode 100644
index 0000000000..de2ee1a71f
--- /dev/null
+++ b/contrib/pg_stat_statements/sql/oldextversions.sql
@@ -0,0 +1,39 @@
+-- test old extension version entry points
+
+CREATE EXTENSION pg_stat_statements WITH VERSION '1.4';
+-- Execution of pg_stat_statements_reset() is granted to pg_read_all_stats
+-- since 1.5, so this fails.
+SET SESSION AUTHORIZATION pg_read_all_stats;
+SELECT pg_stat_statements_reset();
+RESET SESSION AUTHORIZATION;
+
+AlTER EXTENSION pg_stat_statements UPDATE TO '1.5';
+-- Execution of pg_stat_statements_reset() should be granted to
+-- pg_read_all_stats now, so this works.
+SET SESSION AUTHORIZATION pg_read_all_stats;
+SELECT pg_stat_statements_reset();
+RESET SESSION AUTHORIZATION;
+
+-- And in 1.6 it got restricted back to superusers.
+AlTER EXTENSION pg_stat_statements UPDATE TO '1.6';
+SET SESSION AUTHORIZATION pg_read_all_stats;
+SELECT pg_stat_statements_reset();
+RESET SESSION AUTHORIZATION;
+SELECT pg_get_functiondef('pg_stat_statements_reset'::regproc);
+
+-- New function for pg_stat_statements_reset introduced, still
+-- restricted for non-superusers.
+AlTER EXTENSION pg_stat_statements UPDATE TO '1.7';
+SET SESSION AUTHORIZATION pg_read_all_stats;
+SELECT pg_stat_statements_reset();
+RESET SESSION AUTHORIZATION;
+SELECT pg_get_functiondef('pg_stat_statements_reset'::regproc);
+\d pg_stat_statements
+SELECT count(*) > 0 AS has_data FROM pg_stat_statements;
+
+-- New functions and views for pg_stat_statements in 1.8
+AlTER EXTENSION pg_stat_statements UPDATE TO '1.8';
+\d pg_stat_statements
+SELECT pg_get_functiondef('pg_stat_statements_reset'::regproc);
+
+DROP EXTENSION pg_stat_statements;
--
2.33.0
signature.asc
Description: PGP signature
