> On 23.05.2023, at 19:40, Tom Lane <t...@sss.pgh.pa.us> wrote: > > Markus Winand <markus.win...@winand.at> writes: >> I noticed that errors due to writable CTEs in read-only or non-volatile >> context say the offensive command is SELECT. > > Good point. > >> My first thought was that these error messages should mention INSERT, but >> after looking into the source I’m not sure anymore. The name of the command >> is obtained from CreateCommandName(). After briefly looking around it >> doesn’t seem to be trivial to introduce something along the line of >> CreateModifyingCommandName(). > > Yeah, you would have to inspect the plan tree pretty carefully to > determine that. > > Given the way the test is written, maybe it'd make sense to forget about > mentioning the command name, and instead identify the table we are > complaining about: > > ERROR: table "foo" cannot be modified in a read-only transaction
Attached patch takes the active form:
cannot modify table ”foo" in a read-only transaction
It obtains the table name by searching rtable for an RTE_RELATION with
rellockmode == RowExclusiveLock. Not sure if there are any cases where that
falls apart.
> I don't see any huge point in using PreventCommandIfReadOnly if we
> go that way, so no refactoring is needed: just test XactReadOnly
> directly.
As there are several places where this is needed, the patch introduces some
utility functions.
More interestingly, I found that BEGIN ATOMIC bodies of non-volatile functions
happily accept data-modifying statements and FOR UPDATE. While they fail at
runtime it was my expectation that this would be caught at CREATE time. The
attached patch also takes care of this by walking the Query tree and looking
for resultRelation and hasForUpdate — assuming that non-volatile functions
should have neither. Let me know if this is desired behavior or not.
-markus
0001-immutable-stable-create-time-validation.patch
Description: Binary data
