Re: READ ONLY fixes

On Fri, Jan 21, 2011 at 7:08 PM, Kevin Grittner
<Kevin.Grittner@wicourts.gov> wrote:
> Robert Haas <robertmhaas@gmail.com> wrote:
>> Jeff Janes <jeff.janes@gmail.com> wrote:
>>> I found the following message somewhat confusing:
>>> ERROR:  read-only property must be set before any query
>>
>> I think what we need here is two messages, this one and a similar
>> one that starts with "read-write property...".
>
> Done.  I started out by being cute with plugging "only" or "write"
> into a single message, but then figured that might be hard on
> translators; so I went with two separate messages.

Make sense.

I committed the part of this that applies to SET TRANSACTION ISOLATION
LEVEL; the remainder is attached.

Upon further review, I am wondering if it wouldn't be simpler and more
logical to allow idempotent changes of these settings at any time, and
to restrict only changes that actually change something.  It feels
really weird to allow changing these properties to their own values at
any time within a subtransaction, but not in a top-level transaction.
Why not:

if (source != PGC_S_OVERRIDE && newval && XactReadOnly)
{
    if (IsSubTransaction())
        cannot set transaction read-write mode inside a read-only transaction;
    else if (FirstSnapshotSet)
        transaction read-write mode must be set before any query;
    else if (RecoveryInProgress())
        cannot set transaction read-write mode during recovery;
}

That seems a lot more straightforward than this logic, and it saves
one translatable message, too.

I'm not bent on this route if people feel strongly otherwise, but it
seems like it'd be simpler without really losing anything.

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