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

On Wed, Jan 8, 2020 at 3:26 PM Tom Lane <[email protected]> wrote:
> Robert Haas <[email protected]> writes:
> > On Wed, Jan 8, 2020 at 2:57 PM Tom Lane <[email protected]> wrote:
> >> * 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?
>
> COMMAND_NOT_IN_RECOVERY, maybe?

Well, maybe I should just get rid of COMMAND_IS_WEAKLY_READ_ONLY and
return individual COMMAND_OK_IN_* flags for those cases.

> >> * ALTER SYSTEM SET is readonly?  Say what?
>
> > It would be extremely lame and a huge usability regression to
> > arbitrary restrict ALTER SYSTEM SET on standby nodes for no reason.
>
> I didn't say that it shouldn't be allowed on standby nodes.

Oh, OK. I guess I misunderstood.

> I said
> it shouldn't be allowed in transactions that have explicitly declared
> themselves to be read-only.  Maybe we need to disaggregate those
> concepts a bit more --- a refactoring such as this would be a fine
> time to do that.

Yeah, the current rules are pretty weird. Aside from ALTER SYSTEM ..
SET, read-only transaction seem to allow writes to temporary relations
and sequences, plus CLUSTER, REINDEX, VACUUM, PREPARE, ROLLBACK
PREPARED, and COMMIT PREPARED, all of which sound a lot like writes to
me. They also allow LISTEN and SET which are have transactional
behavior in general but for some reason don't feel they need to
respect the R/O property. I worry that if we start whacking these
behaviors around we'll get complaints, so I'm cautious about doing
that. At the least, we would need to have a real clear definition, and
if there is such a definition that covers the current cases, I can't
guess what it is. Forget ALTER SYSTEM for a minute -- why is it OK to
rewrite a table via CLUSTER in a R/O transaction, but not OK to do an
ALTER TABLE that changes the clustering index? Why is it not OK to
LISTEN on a standby (and presumably not get any notifications until a
promotion occurs) but OK to UNLISTEN? Whatever reasons may have
justified the current choice of behaviors are probably lost in the
sands of time; they are for sure unknown to me.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company