Re: Add some tests for pg_stat_statements compatibility verification under contrib

From: Erica Zhang <ericazhangy(at)qq(dot)com>
To: Julien Rouhaud <rjuju123(at)gmail(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Add some tests for pg_stat_statements compatibility verification under contrib
Date: 2021-03-10 06:51:07
Message-ID: tencent_BD2FF6CF7ABA36967937D5D1588D94DD740A@qq.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Julien,
Thanks a lot for the quick review. Please see my answer below in blue. Attached is the new patch.

------------------ Original ------------------
From: "Julien Rouhaud" <rjuju123(at)gmail(dot)com&gt;;
Date:&nbsp;Tue, Mar 9, 2021 05:09 PM
To:&nbsp;"Erica Zhang"<ericazhangy(at)qq(dot)com&gt;;
Cc:&nbsp;"pgsql-hackers"<pgsql-hackers(at)postgresql(dot)org&gt;;
Subject:&nbsp;Re: Add some tests for pg_stat_statements compatibility verification under contrib

Hi,

On Tue, Mar 09, 2021 at 11:35:14AM +0800, Erica Zhang wrote:
&gt; Hi All,
&gt; On the master branch, it is possible to install multiple versions of pg_stat_statements with CREATE EXTENSION, but all the tests in sql/ on look at the latest version available, without testing past compatibility.
&gt;
&gt; Since we support to install lowest version 1.4 currently, add some tests to verify compatibility, upgrade from lower versions of pg_stat_statements.

The upgrade scripts are already tested as postgres will install 1.4 and perform
all upgrades to reach the default version.
Thanks for pointing that the upgrades paths are covered by upgrade scripts tests. So I don't need to verify the upgrade, I will test the installation of different versions directly, any concern?

But an additional thing being tested here is the ABI compatibility when there's
a mismatch between the library and the SQL definition, which seems like a
reasonable thing to test.

Looking at the patch:

+SELECT * FROM pg_available_extensions WHERE name = 'pg_stat_statements' and installed_version = '1.4';

What is this supposed to test?&nbsp; All those tests will break every time we change
the default version, which will add maintenance efforts.&nbsp; It could be good to
have one test breaking when changing the version to remind us to add a test for
the new version, but not more.
Here I just want to verify that "installed" version is the expected version. But we do have the issue as you mentioned which will add maintenance efforts.

So I prefer to keep one test as now which can remind us to add a new version. As for others, just to check the count(*) to make sure installation is success.
Such as SELECT count(*) FROM pg_available_extensions WHERE name = 'pg_stat_statements' and installed_version = '1.4'; What do you think?

Attachment Content-Type Size
v2_add_test_for_pg_stat_statements.patch application/octet-stream 6.4 KB

Browse pgsql-hackers by date

  From Date Subject
Next Message Paul Guo 2021-03-10 06:57:49 Re: Freeze the inserted tuples during CTAS?
Previous Message Michael Paquier 2021-03-10 06:40:38 Re: Occasional tablespace.sql failures in check-world -jnn