All, * Stephen Frost (sfr...@snowman.net) wrote: > Alright, I've got an initial patch to do this for pg_start/stop_backup, > pg_switch_xlog, and pg_create_restore_point. The actual backend changes > are quite small, as expected. I'll add in the changes for the other > functions being discussed and adapt the documentation changes from > the earlier patch to make sense, but what I'd really appreciate are any > comments or thoughts regarding the changes to pg_dump (which are generic > to all of the function changes, of course).
So, I've tested this approach with extensions and binary upgrade and things appear to all be working correctly, but there's a few issues remaining to discuss: The functions pg_start_backup and pg_stop_backup can currently be run by replication roles but those roles won't have any permissions on those functions with the new approach unless we GRANT those rights during pg_dump and/or pg_upgrade. Note that this isn't an issue for tools which use the replication protocol (eg: pg_basebackup) since the replication protocol calls do_pg_start_bacup() directly. As such, my first question is- do we want to worry about this? We should certainly mention it in the release notes but I'm not sure if we really want to do anything else. The second question is in regards to pg_stat_activity. My first suggestion for how to address that would be to have the function return everything and then have the view perform the filtering to return only what's shown today for users. Admins could then grant access to the function for whichever users they want to have access to everything. That strikes me as the best way to avoid changing common usage while still providing the flexibility. Another option would be to still invent the MONITOR role attribute and keep the rest the same. Again, we'd want to mention something in the release notes regarding this change. Lastly, there is the question of pg_cancel_backend and pg_terminate_backend. My thinking on this is to create a new 'pg_signal_backend' which admins could grant access to and leave the existing functions alone (modulo the change for has_privs_of_role as discussed previously). We'd rename the current 'pg_signal_backend' to something else (maybe '_helper'); it's not exposed anywhere and therefore renaming it shouldn't cause any heartache. For pg_create_restore_point, pg_switch_xlog, pg_xlog_replay_pause, pg_xlog_replay_resume, pg_rotate_logfile, we can just remove the if(!superuser()) ereport() checks and REVOKE rights on them during the initdb process, since there isn't anything complicated happening for those today. One other question which I've been thinking about is if we want to try and preserve permission changes associated with other catalog objects (besides functions), or if we even want to change how functions are handled regarding permission changes. We don't currently preserve any such changes across dump/restore or even binary upgrades and this would be changing that. I don't recall any complaints about not preserving permission changes to objects in pg_catalog, but one could argue that our lack of doing so today is a bug. Thoughts? Thanks! Stephen
signature.asc
Description: Digital signature