Re: our checks for read-only queries are not great

On Wed, Jan 8, 2020 at 2:57 PM Tom Lane <tgl@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