On Wed, Jan 8, 2020 at 2:57 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > Hmm. I like the idea of deciding this in one place and insisting that > that one place have a switch case for every statement type. That'll > address the root issue that people fail to think about this when adding > new statements.
Right. Assuming they test their new command at least one time, they should notice. :-) > I'm less enamored of some of the specific decisions here. Notably > > * I find COMMAND_IS_WEAKLY_READ_ONLY to be a more confusing concept > than what it replaces. The test for LockStmt is an example --- the > comment talks about restricting locks during recovery, which is fine and > understandable, but then it's completely unobvious that the actual code > implements that behavior rather than some other one. Uh, suggestions? > * ALTER SYSTEM SET is readonly? Say what? Even if that's how the current > code behaves, it's a damfool idea and we should change it. I think that > the semantics we are really trying to implement for read-only is "has no > effects visible outside the current session" --- this explains, for > example, why copying into a temp table is OK. ALTER SYSTEM SET certainly > isn't that. It would be extremely lame and a huge usability regression to arbitrary restrict ALTER SYSTEM SET on standby nodes for no reason. Editing the postgresql.auto.conf file works just fine there, and is a totally sensible thing to want to do. You could argue for restricting it in parallel mode just out of general paranoia, but I don't favor that approach. > I think if we can sort out the notation for how the restrictions > are expressed, this'll be a good improvement. Thanks. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
