On Tue, Mar 15, 2022 at 4:52 PM Daniel Gustafsson <dan...@yesql.se> wrote: > Yeah, that was the previously posted v25 from the author (who adopted it from > the original author). I took the liberty to quickly poke at the review > comments you had left as well as the ones that I had found to try and progress > the patch. 0001 should really go in it's own thread though to not hide it > from > anyone interested who isn't looking at this thread.
Some comments on 0001: - In general, I think we should prefer to phrase options in terms of what is done, rather than what is not done. For instance, the corresponding GUC for row-level security is row_security={on|off}, not ignore_row_security. - I think it's odd that the GUC in question doesn't accept true and false and our usual synonyms for those values. I suggest that it should, even if we want to add more possible values later. - "ignoreing" is mispleled. So is gux-ignore-event-trigger. "Even triggers" -> "Event triggers". - Perhaps the documentation for the GUC should mention that the GUC is not relevant in single-user mode because event triggers don't fire then anyway. - "Disable event triggers during the session." isn't a very good description because there is in theory nothing to prevent this from being set in postgresql.conf. Basically, I think 0001 is a good idea -- I'm much more nervous about 0002. I think we should get 0001 polished up and committed. -- Robert Haas EDB: http://www.enterprisedb.com