Re: Wrong command name in writeable-CTE related error messages

Поиск
Список
Период
Сортировка
От Markus Winand
Тема Re: Wrong command name in writeable-CTE related error messages
Дата
Msg-id 5C45A8B6-2E34-4012-AF95-9FB09037D2FB@winand.at
обсуждение исходный текст
Ответ на Re: Wrong command name in writeable-CTE related error messages  (Tom Lane <tgl@sss.pgh.pa.us>)
Список pgsql-hackers
> On 23.05.2023, at 19:40, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Markus Winand <markus.winand@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
betrivial 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
thereare 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
andFOR UPDATE. While they fail at runtime it was my expectation that this would be caught at CREATE time. The attached
patchalso takes care of this by walking the Query tree and looking for resultRelation and hasForUpdate — assuming that
non-volatilefunctions should have neither. Let me know if this is desired behavior or not. 

-markus

Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Robert Haas
Дата:
Сообщение: Re: Eliminate redundant tuple visibility check in vacuum
Следующее
От: Bruce Momjian
Дата:
Сообщение: Re: pg_upgrade instructions involving "rsync --size-only" might lead to standby corruption?