Re: PGSERVICEFILE as part of a normal connection string

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

On Mon, Jan 27, 2025 at 2:01 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
> On Thu, Mar 13, 2025 at 08:53:49AM +0900, Ryo Kanbayashi wrote:
> > Putting a bit of context here. Most of the Postgres hackers based in
> > Japan had a meeting last Friday, and Kanbayashi-san has asked me about
> > patches that introduce to simpler code paths in the tree that could be
> > worked on for this release. I've mentioned this thread to him.
> >
> > > Just to let you know, my action is not intended to steal your
> > > contribution but to prevent your good idea from being lost.
> >
> > Authors and reviewers get busy because of life and work matters, and
> > contributions are listed in the commit logs for everybody who
> > participates. If you can help move this patch forward, thanks a lot
> > for the help! IMO, that would be great. The patch set still needs
> > more reorganization and adjustments, but I think that we can get it
> > there

Michael,
CC: Torsten

I reviewed the patch and add some modification described below.

part of https://www.postgresql.org/message-id/Zz2AE7NKKLIZTtEh%40paquier.xyz
> +# This tests "service" and "servicefile"
>
> You are introducing tests for the existing "service", as well as tests
> for the new "servicefile". Could it be possible to split that into
> two patches for clarity? You'd want one to provide coverage for the
> existing features (PGSERVICEFILE, PGSERVICE and connection parameter
> "service"), then add tests for the new feature "servicename" with its
> libpq implementation. That would make your main patch simpler, as
> well.
>
> +open my $fh, '>', $srvfile or die $!;
> +print $fh "[my_srv]\n";
> +print $fh +($node->connstr =~ s/ /\n/gr), "\n";
> +close $fh;
>
> Sure that's OK on Windows where we have CRLFs, not just LFs?

I did...
* Split the patch to two patches
1) regression test of existing features.
2) adding servicefile option feature, its regression test and etc
* Add codes which care new line code of Windows
* Add comments and apply formatter :)

---
Great Regards,
Ryo Kanbayashi

Attachment Content-Type Size
v1-0001-add-regression-test-of-service-option.patch application/octet-stream 2.8 KB
v2-0001-PGSERVICEFILE-as-part-of-the-connection-string.patch application/octet-stream 7.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ilia Evdokimov 2025-03-20 08:47:56 Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment
Previous Message Jakub Wartak 2025-03-20 08:39:32 Re: AIO v2.5