Protocol level transaction support

[LjPalle]

(Only slightly related to: https://github.com/sunng87/pgwire/issues/79)

So I encountered some problems regarding transactions on the network protocol level when communicating with certain clients. I found two places where transactions are relevant in the PostgreSQL protocol.

One is in the ReadyForQuery message which carries a status flag (see https://www.postgresql.org/docs/current/protocol-message-formats.html#PROTOCOL-MESSAGE-FORMATS-READYFORQUERY) - it seems that pgwire currently uses only the "idle" status and the library user cannot change this after e.g. a client sends a "BEGIN" query.

The other place is the tag of the CommandComplete message (see https://www.postgresql.org/docs/current/protocol-message-formats.html#PROTOCOL-MESSAGE-FORMATS-COMMANDCOMPLETE) - currently pgwire always uses the "SELECT" tag whenever rows are returned, but e.g. a fetch may also retrieve rows during a transaction and in this case a "FETCH" tag is expected, so the library user should be able to specify the tag in this case too.

[sunng87]

hello @LjPalle , while it's possible to completely customize on_query of SimpleQueryHandler for that, I agree we can have built-in support for transaction feature on wire protocol.

My proposal is to add new member in our Response enum for both requirements:

pub enum Response<'a> {
    EmptyQuery,
    Query(QueryResponse<'a>, Tag), // For command tag, add a tag field to customize tag name
    Execution(Tag),
    Error(Box<ErrorInfo>),
    TransactionFailed, // return ready state `E`
    TransactionInProgress, // maybe we can have better name, returns ready state `T`
}

An alternative idea for fetch is to add new member called Fetch(QueryResponse<'a>), just depending on how many commands to be included.

I'd like to know your idea for this when you are writing a transaction-enable application based on this library. Also one more question, is it for SimpleQueryHandler only, do we need to support it for ExtendedQueryHandler too?

[LjPalle]

Hey @sunng87, I tweaked pgwire for my purposes in the following way:

• regarding the tag, I added a command_tag String field inside the QueryResponse struct so that inside the send_query_response I could simply use that instead of "SELECT"
• regarding the transaction state I've modified the ClientInfo trait so that there is also a transaction_state getter and setter, and then I've used that wherever ReadyForQuery message was created, instead of using only READY_STATUS_IDLE (the DefaultClient should use it of course as the initial value)

This was an expedient way for me to solve this issue, but I'm not sure whether it is the best. Yeah, I think this should include ExtendedQueryHandler too (my tweak above encompasses this implicitly). I'm not sure where are you aiming at with expanding the Response enum.

[sunng87]

Thank you @LjPalle

For tag inclusion, I think it's totally OK to put a name into QueryResponse. Could you please make a pull request for that?

For transaction state, the reasons why I'm not maintaining it with the client are:

• I think the state transition only happens when a particular query is executed, so the new state can be a return type of a query.
• At the framework level, we don't know the exact time when state will change. It depends on how query is processed. So if we are going to maintain the state with ClientInfo, we will be asking user to keep it update to date accordingly, which makes the implementation complex. For example, we can't say the client state is idle when it's running a query.

But I think it's totally OK for your own application to track it that way since you have full control how the state is used.

[LjPalle]

Yeah sure, I'll send a pull request when I find the time.

Regard the state transition, won't the user need to somehow update or signal it anyway?

[sunng87]

My idea is we only allow user to update it via return value of do_query

[LjPalle]

Ah, ok, now I understand what you meant. But note that one still has to store the transaction state somewhere since the ReadyForQuery message is sent even after parse and describe requests (unless the plan is to change their interface too).

[syedzayyan]

Hi, Is it possible to handle transactions yet? We are playing around with supporting foreign tables and it sends a transaction as follows:

Simple Query Handler:

START TRANSACTION ISOLATION LEVEL REPEATABLE READ;

Extended Query Handler:

describe: Portal { name: "POSTGRESQL_DEFAULT_NAME", statement: StoredStatement { id: "POSTGRESQL_DEFAULT_NAME", statement: "DECLARE c1 CURSOR FOR\nSELECT id, name, ts, signed FROM public.test", parameter_types: [] }, parameter_format: UnifiedText, parameters: [], result_column_format: UnifiedText }

extended query: "DECLARE c1 CURSOR FOR\n  SELECT id, name, ts, signed FROM public.test"

It's unclear what the response to start transaction should be. Investigating for postgres_fdw the cursor response needs to be a non-error so in theory getting past the transaction it should be possible to respond to the fetches.

Any pointers are much appreciated.

[sunng87]

@syedzayyan At the moment, we haven't cover transaction state in this library. You might be able to track it with a custom field in ClientInfo::metadata().

For users who has a scenario for this, contribution or just ideas are welcomed.