Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1mVlYs-0001UL-6S for pgsql-hackers@arkaria.postgresql.org; Thu, 30 Sep 2021 02:12:39 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1mVlYo-0002dU-Tr for pgsql-hackers@arkaria.postgresql.org; Thu, 30 Sep 2021 02:12:34 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1mVlYn-0002dM-Jr for pgsql-hackers@lists.postgresql.org; Thu, 30 Sep 2021 02:12:34 +0000 Received: from wout4-smtp.messagingengine.com ([64.147.123.20]) by magus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1mVlYj-0000bP-MK for pgsql-hackers@postgresql.org; Thu, 30 Sep 2021 02:12:32 +0000 Received: from compute6.internal (compute6.nyi.internal [10.202.2.46]) by mailout.west.internal (Postfix) with ESMTP id A91843200A5F; Wed, 29 Sep 2021 22:12:25 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute6.internal (MEProxy); Wed, 29 Sep 2021 22:12:25 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=paquier.xyz; h= date:from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=fm2; bh=o7ilQB/IWWXcVk7pm5o2W4Xzj3n r5Jw52ChS7s56JVA=; b=ajQ2G4ehtMMZ/R61p1UpJwqiP43psNaDoIcfqn9A1QL ONI9CIFUuRuuNOBHKBOxKDSSl2wJ5TQz6VL4y6tYyERT0MtmHUxZVZrznJL1xRfJ W/89VlBRjXJTq9sxOJYJECB99thjdBmExl6NxBIv0IENsDFocGhEqNtMCKb4zs9P ClQ3J3PWT1ft2uh2HUvW2lDcOPUUvmwhq05FJkXzUAxuyLKrQnl8m+SpLtnOcHr/ +9ZgNGM3uhmhHRQzw2FxZ2J/7eTSU95xbdHvVR2Wj2PFnUepFpHwdcVb/bfzRhDS rYIcCNJGdrIEX42ESmu9xdO6O5rdwV0QAGBdkBqvmOw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm3; bh=o7ilQB /IWWXcVk7pm5o2W4Xzj3nr5Jw52ChS7s56JVA=; b=hWRMg0a2OAyt+eF8Y3aq6l 4ni4Ry2oN0Np5x7ogXcDhFr8lh1dzEwA9PerxeIDyoIN00wYrG1FQtQfFzDdCtED dQYtio0ubIWEZ5hwJAel/Xhl2QL9WgoHb8R7WxZyXK4HLLsP8Ojrh+vw1Nx+lnHb aR4VcrZDSIEL/rYje03MUvM8diuaTSzW+RIu1gQedaLL3AUS5A5QBl8EmJabB6MK X1PhBzs8QgOwAEPj4ifOLocn56H/Nbezwdp4egDpTV1LQD9PiWsJDZMeOEMkvY9D 4SpkSviw8dteN34cxba453/rkp9Ky9uQPTdtBcVQxjPNB25/PvK5w2Pj+69jvvfQ == X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvtddrudekfedghedvucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucgfrhhlucfvnfffucdljedtmdenucfjughrpeffhf fvuffkfhggtggujgesghdtreertddtvdenucfhrhhomhepofhitghhrggvlhcurfgrqhhu ihgvrhcuoehmihgthhgrvghlsehprghquhhivghrrdighiiiqeenucggtffrrghtthgvrh hnpedvgeduuefhtdeuleettdevjeehheeiveeuieegleetgeeljeelieeuieehgeevhfen ucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehmihgthh grvghlsehprghquhhivghrrdighiii X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 29 Sep 2021 22:12:23 -0400 (EDT) Date: Thu, 30 Sep 2021 11:12:21 +0900 From: Michael Paquier To: Erica Zhang Cc: Julien Rouhaud , pgsql-hackers Subject: Re: Add some tests for pg_stat_statements compatibility verification under contrib Message-ID: References: <20210309090937.4eca74eme7jvcvhi@nol> <20210310033553.v5ajijyswer4ux5q@nol> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="vp53q7s+ErbEEUU0" Content-Disposition: inline In-Reply-To: List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --vp53q7s+ErbEEUU0 Content-Type: multipart/mixed; boundary="WBEJtM310TOVmZYz" Content-Disposition: inline --WBEJtM310TOVmZYz Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 --WBEJtM310TOVmZYz Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v4-0001-Add-some-tests-for-past-versions-of-pg_stat_state.patch" Content-Transfer-Encoding: quoted-printable =46rom 9742f2a8b717310ff75ac061f274cbc0502ac8b8 Mon Sep 17 00:00:00 2001 =46rom: Michael Paquier 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_statemen= ts/Makefile index 3ec627b956..7fabd96f38 100644 --- a/contrib/pg_stat_statements/Makefile +++ b/contrib/pg_stat_statements/Makefile @@ -16,7 +16,7 @@ PGFILEDESC =3D "pg_stat_statements - execution statistics= of SQL statements" LDFLAGS_SL +=3D $(filter -lm, $(LIBS)) =20 REGRESS_OPTS =3D --temp-config $(top_srcdir)/contrib/pg_stat_statements/pg= _stat_statements.conf -REGRESS =3D pg_stat_statements +REGRESS =3D pg_stat_statements oldextversions # Disabled because these tests require "shared_preload_libraries=3Dpg_stat= _statements", # which typical installcheck users do not have (e.g. buildfarm clients). NO_INSTALLCHECK =3D 1 diff --git a/contrib/pg_stat_statements/expected/oldextversions.out b/contr= ib/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=20 +-------------------------- +=20 +(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 = =20 +--------------------------------------------------------------------------= ----- + 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$funct= ion$+ +=20 +(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 = =20 +--------------------------------------------------------------------------= ------------------------------------------------------ + CREATE OR REPLACE FUNCTION public.pg_stat_statements_reset(userid oid DEF= AULT 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$f= unction$ + +=20 +(1 row) + +\d pg_stat_statements + View "public.pg_stat_statements" + Column | Type | Collation | Nullable | Default= =20 +---------------------+------------------+-----------+----------+--------- + userid | oid | | |=20 + dbid | oid | | |=20 + queryid | bigint | | |=20 + query | text | | |=20 + calls | bigint | | |=20 + total_time | double precision | | |=20 + min_time | double precision | | |=20 + max_time | double precision | | |=20 + mean_time | double precision | | |=20 + stddev_time | double precision | | |=20 + rows | bigint | | |=20 + shared_blks_hit | bigint | | |=20 + shared_blks_read | bigint | | |=20 + shared_blks_dirtied | bigint | | |=20 + shared_blks_written | bigint | | |=20 + local_blks_hit | bigint | | |=20 + local_blks_read | bigint | | |=20 + local_blks_dirtied | bigint | | |=20 + local_blks_written | bigint | | |=20 + temp_blks_read | bigint | | |=20 + temp_blks_written | bigint | | |=20 + blk_read_time | double precision | | |=20 + blk_write_time | double precision | | |=20 + +SELECT count(*) > 0 AS has_data FROM pg_stat_statements; + has_data=20 +---------- + 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= =20 +---------------------+------------------+-----------+----------+--------- + userid | oid | | |=20 + dbid | oid | | |=20 + queryid | bigint | | |=20 + query | text | | |=20 + plans | bigint | | |=20 + total_plan_time | double precision | | |=20 + min_plan_time | double precision | | |=20 + max_plan_time | double precision | | |=20 + mean_plan_time | double precision | | |=20 + stddev_plan_time | double precision | | |=20 + calls | bigint | | |=20 + total_exec_time | double precision | | |=20 + min_exec_time | double precision | | |=20 + max_exec_time | double precision | | |=20 + mean_exec_time | double precision | | |=20 + stddev_exec_time | double precision | | |=20 + rows | bigint | | |=20 + shared_blks_hit | bigint | | |=20 + shared_blks_read | bigint | | |=20 + shared_blks_dirtied | bigint | | |=20 + shared_blks_written | bigint | | |=20 + local_blks_hit | bigint | | |=20 + local_blks_read | bigint | | |=20 + local_blks_dirtied | bigint | | |=20 + local_blks_written | bigint | | |=20 + temp_blks_read | bigint | | |=20 + temp_blks_written | bigint | | |=20 + blk_read_time | double precision | | |=20 + blk_write_time | double precision | | |=20 + wal_records | bigint | | |=20 + wal_fpi | bigint | | |=20 + wal_bytes | numeric | | |=20 + +SELECT pg_get_functiondef('pg_stat_statements_reset'::regproc); + pg_get_functiondef = =20 +--------------------------------------------------------------------------= ------------------------------------------------------ + CREATE OR REPLACE FUNCTION public.pg_stat_statements_reset(userid oid DEF= AULT 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$f= unction$ + +=20 +(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; --=20 2.33.0 --WBEJtM310TOVmZYz-- --vp53q7s+ErbEEUU0 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmFVHQUACgkQnvQgOdby QH2YlQ/+M/JnnYnqaKNV/YJgmMYtoJxLYTahe2qfH4vmWRkduNX6Pw3OQcEYDGzN p5inbWkjOhpfL+ZHO9zYNh0Vk1o4Oq45P++jKLBUTtVJEZ8/FJp/cpUol0i4M1XO zvaDDobMtPCToo3MZpDTp0+oTLQJMaYhnUpfr6lE5cUBfXgo+1EKNqBSyJoCmobx 7jcWXTT6hnnrOy99ih+IOaDxC8SX0zkhG079tkismuq6CYtjD9BEJOle3lR3TO63 naI1CUz1tZPzUh189SXb0jjTyPN2b8c5XXwP7d9DBJh9bWL9FKYHYGn0/OQON2cO Z/3HzR+1VropBlh6lqmAj6cr1iamQausYpqTrGRraXO7t7Hv3vHHYHwsoAo8irrH dGv2pL/bt6aKItdaoh40VRB/twDuB9LDehQ9vjz1jxdnIBxThoCQ3FR2XD/v+BxN Wg95MjTJ1Havx5Ps3R5CJLsqgpv/FFQ44fkqnZhd2SK0QmBz2bmquWYkGKqcHbdj oDfWCV1Z2NKx4IGe7uCgZN3VDaB2QdRg5HeEgqJtfzecDxH9EOVeqmQpUsdxz6D6 G3SSgTAGgutoWM/U4ajFuYnfLFf66kIvKDw+s9koSkCxOGw4zRNAL16/yb0RjkQi qh6NLdasDuxh4qjff7GPJI4pUNI6kLmsTt6ygzK2DWSQeVmJvUA= =s3mi -----END PGP SIGNATURE----- --vp53q7s+ErbEEUU0--