On Wed, Jun 18, 2025 at 12:23 PM Michael Paquier <michael@paquier.xyz> wrote:
> 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.
OK.
I think I was able to fix it as per your request :)
> 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.
No problem.
I've attached modified and splited patch files to this mail.
---
Great regards,
Ryo Kanbayashi
