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

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> I spent some time studying the question of how we classify queries as
> either read-only or not, and our various definitions of read-only, and
> found some bugs. ...
> However, I think that all of them can be tracked back to a more
> fundamental underlying cause, which is that the way that the various
> restrictions on read-write queries are implemented is pretty
> confusing. Attached is a patch I wrote to try to improve things.

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.

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.

* 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.

I haven't read all of the code; those were just a couple points that
jumped out at me.

I think if we can sort out the notation for how the restrictions
are expressed, this'll be a good improvement.

regards, tom lane