Another thing that should be described there is that this falls outside of the transaction flow, i.e. it's changes are not reverted on ROLLBACK. But that leaves an important consideration: What happens when an error occurs on the server during handling of this message (e.g. the GUC does not exist or an OOM is triggered). Is any open transaction aborted in that case? If not, we should have a test for that.
I tested this scenario. I had to modify message handling to fix warning "message type 0x5a arrived from server while idle"
I fixed this issue. The problem was in the missing setting `doing_extended_query_message`.
But if this is inside a transaction, the transaction is aborted.
+ if (PQresultStatus(res) != PGRES_COMMAND_OK) + pg_fatal("failed to link custom variable: %s", PQerrorMessage(conn)); + PQclear(res);
done
The tests should also change the config after running PQlinkParameterStatus/PQunlinkParameterStatus to show that the guc is reported then or not reported then.
done
+ if (!PQsendTypedCommand(conn, PqMsg_ReportGUC, 't', paramName)) + return NULL;
I think we'll need some bikeshedding on what the protocol message should look like exactly. I'm not entirely sure what is the most sensible here, so please treat everything I write next as suggestions/discussion: I see that you're piggy-backing on PQsendTypedCommand, which seems nice to avoid code duplication. It has one downside though: not every type, is valid for each command anymore. One way to avoid that would be to not introduce a new command, but only add a new type that is understood by Describe and Close, e.g. a 'G' (for GUC). Then PqMsg_Describe, G would become the equivalent of what'the current patch its PqMsg_ReportGUC, 't' and PqMsg_Close, G would be the same as PqMsg_ReportGUC, 'f'.
I am sorry, I don't understand this idea?
The rest of this email assumes that we're keeping your current proposal for the protocol message, so it might not make sense to address all of this feedback, in case we're still going to change the protocol:
