Timeout logic fails with errors within transactions

[okeuday]

I was testing using pgsql_connection:simple_query/4 with a sequence of SQL commands that create a transaction (BEGIN/(COMMIT|ROLLBACK)) and found I was able to crash the pgsql_connection pid at https://github.com/semiocast/pgsql/blob/master/src/pgsql_connection.erl#L699 due to the fun argument not handling "{error, {pgsql_error, [...proplist data...]}}" (i.e., instead it expects to always get "{set, []}"). This occurred when one of the SQL statements supplied was invalid. This should be an error that is not yet handled within pgsql_connection, since the Erlang pid should not be dying in this case.

[semiocast]

Thank you for the bug report and analysis. This bug was fixed in 693958e.

[okeuday]

Thank you for the fix!

[okeuday]

After some more testing, it seems like this fix isn't complete. When a transaction dies before the COMMIT command another simple_query function call with a ROLLBACK command should be possible, however, it doesn't work when using this driver. For some reason, the driver locks onto the last error state and doesn't allow the ROLLBACK to occur.

[pguyot]

Michael,

Thank you for this additional report. I reviewed and fixed the logic. New tests were added corresponding to your description. Indeed, it seems you have been issuing ROLLBACK commands with a non-default (infinity) timeout.

Logic could be optimized eventually around your usage pattern if the specified timeout is always the same, as every statement with a timeout is sent with two additional statements (SET and RESET). However, this doesn't sound obvious as transaction failures resets the statement_timeout run-time parameter.

[okeuday]

Thanks, that fixed it.