Message-ID: <1f84685f-1ffd-4937-de63-aaa1513a0b7e@gmail.com>
Date: Thu, 2 Mar 2023 08:12:24 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.8.0
Subject: Re: Normalization of utility queries in pg_stat_statements
To: Michael Paquier <michael@paquier.xyz>
Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>
References: <Y+MRdEq9W9XVa2AB@paquier.xyz> <Y+16K+q9L0yjV3zA@paquier.xyz>
 <0e067034-e92c-302b-24ed-492bca228e26@gmail.com>
 <Y+7n+uz6XkzJpB7z@paquier.xyz>
 <14965f63-6d78-45ff-d029-daafe8eddb40@gmail.com>
 <Y/Lbt12YUHSU2fYl@paquier.xyz> <Y/LcU/u8CbD+nxYp@paquier.xyz>
 <Y/7Y9U/y/keAW3qH@paquier.xyz>
Content-Language: en-US
From: "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com>
In-Reply-To: <Y/7Y9U/y/keAW3qH@paquier.xyz>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/1f84685f-1ffd-4937-de63-aaa1513a0b7e%40gmail.com>
Precedence: bulk

Hi,

On 3/1/23 5:47 AM, Michael Paquier wrote:
> On Mon, Feb 20, 2023 at 11:34:59AM +0900, Michael Paquier wrote:
>> With the patches..
> 
> Attached is an updated patch set, where I have done more refactoring
> work for the regression tests of pg_stat_statements, splitting
> pg_stat_statments.sql into the following files:
> - user_activity.sql for the role-level resets.
> - wal.sql for the WAL generation tracking.
> - dml.sql for insert/update/delete/merge and row counts.
> - The main file is renamed to select.sql, as it now only covers SELECT
> patterns.
> 

Thanks!

Splitting even more and removing pg_stat_statements.sql/out does make sense to me,
so +1 for the patch.

Applying 0001 produces:

Applying: Split more regression tests of pg_stat_statements
.git/rebase-apply/patch:1735: new blank line at EOF.
+
.git/rebase-apply/patch:2264: new blank line at EOF.
+
warning: 2 lines add whitespace errors.


Nits:

+++ b/contrib/pg_stat_statements/sql/wal.sql
@@ -0,0 +1,22 @@
+--
+-- Validate WAL generation metrics
+--
+
+SET pg_stat_statements.track_utility = FALSE;
+
+-- utility "create table" should not be shown

This comment is coming from the previous pg_stat_statements.sql but
I wonder if it makes sense here as testing utility is not the initial purpose
of wal.sql.

Same comment for dml.sql:

+-- utility "create table" should not be shown
+CREATE TEMP TABLE pgss_dml_tab (a int, b char(20));

What about removing those comments?

> There is no change in the code coverage or the patterns tested.

I had a look (comparing the new .sql files with the old pg_stat_statements.sql content) and I agree.

Except from the Nits above, 0001 LGTM.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com