On Wed, Jun 18, 2025 at 12:23 PM Michael Paquier <mich...@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

Attachment: v11-0001-add-test-for-nested-options-in-servie-file.patch
Description: Binary data

Attachment: v11-0002-servicefile-option-usage-on-connection-string-fe.patch
Description: Binary data

Attachment: v11-0003-psql-enhancement-related-servicefile-option-on-c.patch
Description: Binary data

Reply via email to