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

On Wed, Jan 8, 2020 at 2:57 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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