Thread: [subxact] Proof-of-concept: report nest level to client

Hackers,

I tried to implement the reporting of the current transaction level to
the client using an ad-hoc ParameterStatus message.  Seems it works.
To see it working I added a %d escape to psql prompt processing:

alvherre=# \set PROMPT1 '%n@%/%R[%d]%x '
alvherre@alvherre=[0] begin;
BEGIN
alvherre@alvherre=[1]* savepoint foo;
SAVEPOINT
alvherre@alvherre=[2]* savepoint bar;
SAVEPOINT
alvherre@alvherre=[3]* release foo;
RELEASE
alvherre@alvherre=[1]* savepoint another;
SAVEPOINT
alvherre@alvherre=[2]* commit;
COMMIT
alvherre@alvherre=[0]


(The nesting level, obviously, is the number between [ ]; the * is the
"in transaction" mark, which existed previously.  Yes, it works if it's
> 9.)

Patch attached (surprinsingly small), though it only applies with the
savepoint patch applied(*).  If any driver writer wants to play, however,
it's easy to see what's going on -- a ParameterStatus message will be
received from the backend whenever the nesting level changes.

I added a function PQnestingLevel() to libpq, and a corresponding field
in pg_conn.  We have to decide if we like the name, and whether we want
to have it at all.

(This is different from the previous idea in that the nesting level is
not a GUC variable -- the message is sent directly from xact.c.  If this
is a bad idea, just moving the SendTransactionNestingLevel() function
can be moved somewhere else, though I couldn't figure out where.)



(*)  Even then, this is hand-edited output of interdiff, so maybe it
doesn't apply at all ... if this is the case I'll submit a better patch
tomorrow.

--
Alvaro Herrera (<alvherre[a]dcc.uchile.cl>)
"No hay hombre que no aspire a la plenitud, es decir,
la suma de experiencias de que un hombre es capaz"

Alvaro Herrera <alvherre@dcc.uchile.cl> writes:
> Patch attached (surprinsingly small), though it only applies with the
> savepoint patch applied(*).  If any driver writer wants to play, however,
> it's easy to see what's going on -- a ParameterStatus message will be
> received from the backend whenever the nesting level changes.

This is really quite the wrong behavior --- think about a function that
executes thousands of subtransactions before returning.  You really do
not want to emit a message for every nestlevel change.  (That's one of
the reasons I rejected doing this as a regular GUC variable.)

We could possibly send the info as a fake ParameterStatus that's only
sent just before the Z message, ie only when the backend is about to
wait for a command.

Right at the moment though I'm suspending judgment on whether we should
bite the bullet and do a protocol version change to support nested
xacts.  Off the top of my head we have a couple of reasons to want one:
- need for xact nest level or something like it in Z message
- need for option to make protocol-created portals act like WITH HOLD cursors
and IIRC we have identified some other minor misfeatures in the 7.4
protocol design as well, though I'd have to dig through the archives
to remember the details.  If we find any more I'll start to think that
we should just do it ...

            regards, tom lane

Tom Lane wrote:
> Alvaro Herrera <alvherre@dcc.uchile.cl> writes:
>
>>Patch attached (surprinsingly small), though it only applies with the
>>savepoint patch applied(*).  If any driver writer wants to play, however,
>>it's easy to see what's going on -- a ParameterStatus message will be
>>received from the backend whenever the nesting level changes.
>
>
> This is really quite the wrong behavior --- think about a function that
> executes thousands of subtransactions before returning.  You really do
> not want to emit a message for every nestlevel change.  (That's one of
> the reasons I rejected doing this as a regular GUC variable.)
>
> We could possibly send the info as a fake ParameterStatus that's only
> sent just before the Z message, ie only when the backend is about to
> wait for a command.

This is not really sufficient when using the extended query protocol to
run several queries before a Sync. Consider "SAVEPOINT foo; SELECT ...;
SAVEPOINT bar; SELECT ..." run as (Parse/Bind/Execute)x4 + Sync. If the
client wants to track the nesting level of portals returned by the Bind
messages -- so it can know when they're invalidated on rollback -- it
can't if the level is only given in ReadyForQuery.

What about emitting any nesting level change once at the end of
processing each protocol message? More specifically I think Execute and
Query are the only messages that should change nesting level?

> - need for option to make protocol-created portals act like WITH HOLD cursors

And SCROLL.

> and IIRC we have identified some other minor misfeatures in the 7.4
> protocol design as well, though I'd have to dig through the archives
> to remember the details.  If we find any more I'll start to think that
> we should just do it ...

One change I thought might be useful is to include the name of the
opened or closed portal or statement in ParseComplete, BindComplete,
CloseComplete messages. A V3 client that's streaming multiple commands
before a Sync has to do some extra work to track exactly which resources
it's allocated, remembering the exact order it requested operations and
matching them up to the (anonymous) responses it gets.

Also on my wishlist is a message whenever a portal is implicitly closed
(perhaps a spontaneous CloseComplete?) although I have no idea how hard
this is to implement :)

There was also a brief discussion earlier around defining exactly when
in the protocol stream the client encoding changes after a SET
client_encoding (and the encoding of the startup packets). This might
just be documentation.

But none of these are important enough to force a protocol change by
themselves.

-O