From: | Alexandra Ryzhevich <aryzhevich(at)google(dot)com> |
---|---|
To: | michael(at)paquier(dot)xyz |
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-03 13:08:27 |
Message-ID: | CAOt4E5RLNf+LqysoXyg+fOBuM-bf8z77UVBauSbSkOyTgivrsw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Thank you for pointing to some problems of the patch. I've attached a
modified version below.
On Thu, Aug 2, 2018 at 8:38 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
> On Thu, Aug 02, 2018 at 06:25:14PM +0100, Alexandra Ryzhevich wrote:
> > I have noticed that there is no regression tests for default monitoring
> > roles such as pg_read_all_stats.
>
> A bug has been recently fixed for that, see 0c8910a0, so having more
> coverage would be welcome, now your patch has a couple of problems.
> 25fff40 is the original commit which has introduced pg_read_all_stats.
>
> Your patch has a couple of problems by the way:
> - database creation is costly, those should not be part of the main
> regression test suite.
> - Any roles created should use "regress_" as prefix.
> - You should look also at negative tests which trigger "must be
> superuser or a member of pg_read_all_settings", like say a "SHOW
> stats_temp_directory" with a non-privileged user.
> --
> Michael
>
Attachment | Content-Type | Size |
---|---|---|
0001-Add-regress-test-for-pg_read_all_stats-role.patch | text/x-patch | 7.7 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Jesper Pedersen | 2018-08-03 13:11:50 | Re: partition tree inspection functions |
Previous Message | Fabien COELHO | 2018-08-03 13:04:32 | Re: doc - add missing documentation for "acldefault" |