Greeting, On Wed, Oct 13, 2021 at 04:14 Bharath Rupireddy < bharath.rupireddyforpostg...@gmail.com> wrote:
> On Wed, Oct 13, 2021 at 1:24 PM Michael Paquier <mich...@paquier.xyz> > wrote: > > > > On Wed, Oct 13, 2021 at 11:15:16AM +0530, Bharath Rupireddy wrote: > > > IMO, we can just retain the "if (!superuser())" check in the > > > pg_log_backend_memory_contexts as is. This would be more meaningful as > > > the error "must be superuser to use raw page functions" explicitly > > > says that a superuser is allowed. Whereas if we revoke the permissions > > > in system_views.sql, then the error we get is not meaningful as the > > > error "permission denied for function pg_log_backend_memory_contexts" > > > says that permissions denied and the user will have to look at the > > > documentation for what permissions this function requires. > > > > I don't really buy this argument with the "superuser" error message. > > When removing hardcoded superuser(), we just close the gap by adding > > in the documentation that the function execution can be granted > > afterwards. And nobody has complained about the difference in error > > message AFAIK. That's about extensibility. > > I'm not against removing superuser() check in the > pg_log_backend_memory_contexts. However, there are a lot of functions > with the "must be superuser to XXXXX" kind of error [1]. I'm worried > if someone proposes to change these as well with what we do for > pg_log_backend_memory_contexts. > > brin_page_type > brin_page_items > brin_metapage_info > brin_revmap_data > bt_page_stats_internal > bt_page_items_internal > bt_page_items_bytea > bt_metap > fsm_page_contents > gin_metapage_info > gin_page_opaque_info > and the list goes on. Yes, would generally be good to change at least some of those also, perhaps all of them. Not sure I see what the argument here is. We should really be trying to move away from explicit superuser checks. Thanks. Stephen >
