Re: proposal: psql: show current user in prompt

On Tue, 5 Sept 2023 at 05:50, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:

> I prefer to introduce a new function - it is ten lines of code. The form is not important - it can be a full number or minor number. It doesn't matter I think. But my opinion in this area is not strong, and I like to see feedback from others too. It is true that this feature and interface is not fully complete.

I think when using the API it is nicest to have a single function that
returns the full version number. i.e. if we're introducing a new
function I think it should be PQprotocolFullVersion instead of
PQprotocolSubversion. Then instead of doing a version check like this:

if (PQprotocolVersion(pset.db) == 3 && PQprotocolSubversion(pset.db) >= 1)

You can do the simpler:

if (PQprotocolFullVersion(pset.db) >= 301)

This is also in line with how you do version checks for postgres versions.

So I think this patch should at least add that one instead of
PQprotocolSubversion. If we then decide to replace PQprotocolVersion
with this new implementation, that would be a trivial change.

>> + /* The protocol 3.0 is required */
>> + if (PG_PROTOCOL_MAJOR(their_version) == 3)
>> + conn->pversion = their_version;
>>
>> Let's compare against the actual PG_PROTOCOL_EARLIEST and
>> PG_PROTOCOL_LATEST to determine if the version is supported or not.
>
>
> changed

I think we should also check the minor version part. So like this instead
+ if (their_version < PG_PROTOCOL_EARLIEST || their_version >
PG_PROTOCOL_LATEST)

PS. If you use the -v flag of git format-patch a version number is
prepended to your patches. That makes it easier to reference them.