Re: proposal: psql: show current user in prompt

pá 8. 9. 2023 v 21:07 odesílatel Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
napsal:

> Hi
>
> 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:
>>
>> + if (is_set == 't')
>> + {
>> + SetGUCOptionFlag(name, GUC_REPORT);
>> + status = "SET REPORT_GUC";
>> + }
>> + else
>> + {
>> + UnsetGUCOptionFlag(name, GUC_REPORT);
>> + status = "UNSET REPORT_GUC";
>> + }
>>
>> I think we should be strict about what we accept here. Any other value
>> than 'f'/'t' for is_set should result in an error imho.
>>
>
> done
>

Regards

Pavel

>
>