Re: [PATCH] Add regress test for pg_read_all_stats role

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Alexandra Ryzhevich <aryzhevich(at)google(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org, Vladimir Rusinov <vrusinov(at)google(dot)com>, Dmitriy Potapov <atomsk(at)google(dot)com>
Subject: Re: [PATCH] Add regress test for pg_read_all_stats role
Date: 2018-08-23 08:12:33
Message-ID: 20180823081233.GB30782@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Aug 21, 2018 at 05:48:49PM +0100, Alexandra Ryzhevich wrote:
> Just to check if changes broke something. I haven't find these checks in
> other regress tests. In other way we get only positive tests. If this
> is not needed then should I remove all the checks for
> regress_role_nopriv role or negative regress_role_nopriv tests only?

+-- should not fail because regress_role_haspriv is a member of
pg_read_all_stats
+SELECT pg_database_size('regression') > 0 AS canread;
What is really disturbing about the ones testing the size functions is
that they may be costly when running installcheck. By the way, it would
be nice to avoid the system-wide REVOKE, which could impact any tests
running in parallel. You could simply check for some other NULL
values.

> Changed to use session_preload_libraries.

+-- session_preload_libraries is of PGC_SUSET category
+SET LOCAL session_preload_libraries TO '/tmp/';
+-- vacuum_cost_delay is of PGC_USERSET category
+SET LOCAL vacuum_cost_delay TO 40;

This gets better, thanks. I would suggest using a less realistic value
than "/tmp/", which could become a security hole if copy-pasted around...
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fabien COELHO 2018-08-23 08:17:28 Re: [HACKERS] proposal: schema variables
Previous Message Alexander Kukushkin 2018-08-23 08:01:08 Re: BUG #15346: Replica fails to start after the crash