On Wed, Oct 13, 2021 at 2:19 PM Stephen Frost <sfr...@snowman.net> wrote: > > 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.
Hm. Let's deal with it separately, if required. > Not sure I see what the argument here is. We should really be trying to move > away from explicit superuser checks. I will remove the superuser() for pg_log_backend_memory_context alone here in the next version of patch. Regards, Bharath Rupireddy.
