Re: [PATCH] Add regress test for pg_read_all_stats role

>
> - There is no need for the initial DROP ROLE commands as those already
> get dropped at the end of the tests.
>
Removed.

- There is already rolenames.sql which has a tiny coverage for default
> roles, why not just using it?
>
Moved changes to rolenames.sql.

> +-- should fail because regress_role_nopriv has not CONNECT permission
> on this db
> +SELECT pg_database_size('regression') > 0 AS canread;
> +ERROR: permission denied for database regression
> +-- should fail because regress_role_nopriv has not CREATE permission on
> this tablespace
> +SELECT pg_tablespace_size('pg_global') > 0 AS canread;
> +ERROR: permission denied for tablespace pg_global
> Why is that part of a test suite for default roles?
>
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?

2) is easy to be triggered as a negative test (which fails), less as a
> positive test. In order to make a positive test failure-proof with
> installcheck you would need to have a parameter which can be changed by
> a superuser at session level which gets updated to a certain value, and
> would fail to show for another user, so you could use one which is
> GUC_SUPERUSER_ONLY and of category PGC_SUSET, like
> session_preload_libraries or dynamic_preload_libraries. Still that's
> pretty restrictive, and would only test one out of the three code paths
> available.
>
Changed to use session_preload_libraries.

Alexandra