Re: [PATCH] Add support for displaying database service in psql prompt

On Mon, Jul 07, 2025 at 11:06:06AM +0900, Michael Paquier wrote:
> On Sun, Jul 06, 2025 at 09:13:19AM -0700, Noah Misch wrote:
> > This new PQservice() function from commit 4b99fed75 came up in the annual
> > exports.txt diff. The standard in libpq has been to not clutter the API with
> > new functions that simply retrieve one PQconninfoOption value. PQconninfo()
> > provides access to all those values in a generic way. What do you think of
> > making psql use PQconninfo() for this, then removing PQservice()? The rest of
> > the commit (adding the struct field, necessary for PQconninfo() to include the
> > value) looks good.
>
> Sure, I was not aware of such a policy. Relying on PQconninfoOption
> is less efficient because we would need to look through the whole set
> of options when looking for the service name, and this needs one extra
> allocation as PQconninfoFree() frees the values allocated. With two
> callers perhaps this inefficiency is OK to live with anyway.

I think the choice to make there is whether to call PQconninfo() once per
prompt emission or to cache the value, invalidating that cache e.g. once per
SyncVariables(). My first thought was to cache, but the decision is not too
important. A PQconninfo() call is likely negligible relative to all that
happens between prompts. Even if not negligible, the overhead of not caching
will affect only prompts using the new escape sequence.