Re: [PATCH] PGSERVICEFILE as part of a normal connection string

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Ryo Kanbayashi <kanbayashi(dot)dev(at)gmail(dot)com>
Cc: Torsten Förtsch <tfoertsch123(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: [PATCH] PGSERVICEFILE as part of a normal connection string
Date: 2025-06-18 03:23:00
Message-ID: aFIxFBQJI7g42lQB@paquier.xyz
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sun, Jun 15, 2025 at 09:02:31PM +0900, Ryo Kanbayashi wrote:
> Thanks for review :)

Thanks for the new patch.

While testing the patch, I've bumped into this scenario which feels
incomplete:
- Rely on a default location of the service file, like
$HOME/.pg_service.conf.
- Define a service, with PGSERVICE or a connection parameter.
In this case, :SERVICE shows up some information, not :SERVICEFILE
because it remains empty when building a connection file path if we
don't provide PGSERVICEFILE or servicefile as connection option. It
seems to me that we had better force pg_conn->pgservicefile into a
value in this case, pointing to the value libpq thinks is the default
at the time of resolving the HOME location in pqGetHomeDirectory()?
It seems to me that you should be able to do that at the end of
parseServiceFile(), at least, if we know that the status is a success
(free value if any, assign the new one, and invent an error code path
for the OOM on strdup()).

Defining PGSERVICEFILE or servicefile in a connection string reports
correctly "pgservicefile" in the libpq connection, of course. That's
just for the default location paths.

By the way, could you split the test case for the nested "service"
value in a service file into its own file? This is an existing error
case, and there is no need for the new feature to add this test.
--
Michael

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2025-06-18 03:58:16 Re: Huge commitfest app update upcoming: Tags, Draft CF, Help page, and automated commitfest creat/open/close
Previous Message shveta malik 2025-06-18 03:22:36 Re: Replication slot is not able to sync up