public inbox for [email protected]  

help / color / mirror / Atom feed

From: Steven Niu <[email protected]>
To: Fujii Masao <[email protected]>
To: Álvaro Herrera <[email protected]>
Cc: Shinya Kato <[email protected]>
Cc: Chao Li <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: file_fdw: Support multi-line HEADER option.
Date: Wed, 21 Jan 2026 08:18:41 +0000
Message-ID: <MN2PR15MB3021F257D141B286206F0DC6A796A@MN2PR15MB3021.namprd15.prod.outlook.com> (raw)
In-Reply-To: <CAHGQGwH3PJ8an=Z-70s3Nv=YY3btAJ=qD76Tivh2XgnABq8Xmg@mail.gmail.com>
References: <CAHGQGwGWyApGA0L-uE819xeinwSonPXQfV1diBUCkR+AcgxuOw@mail.gmail.com>
	<[email protected]>
	<CAHGQGwH3PJ8an=Z-70s3Nv=YY3btAJ=qD76Tivh2XgnABq8Xmg@mail.gmail.com>

From: Fujii Masao <[email protected]>
Sent: Wednesday, January 21, 2026 15:38
To: Álvaro Herrera <[email protected]>
Cc: Shinya Kato <[email protected]>; Chao Li <[email protected]>; PostgreSQL-development <[email protected]>
Subject: Re: file_fdw: Support multi-line HEADER option.


On Wed, Jan 21, 2026 at 12:07 AM Álvaro Herrera <[email protected]> wrote:
>
> On 2026-Jan-20, Fujii Masao wrote:
>
> > I'm thinking to commit the attached patches.
>
> In 0001 I would recommend to move the string "match" to outside the
> message.  Otherwise a translator might be tempted to translate it, and
> then it would make no sense.  (I have never seen this happen in
> Postgres, but I have seen this kind of translation mistake in other
> software, and it is infuriating.)

Thanks for the review!

Agreed. I've updated the patches as suggested.
The revised versions are attached.

Regards,

-- 
Fujii Masao



________________________________________



Hi Fujii-san, 

I've noticed an inconsistent naming convention in the patch: 
we use a capitalized Boolean for the boolean type, but a lowercase integer when referring to the value type. 

+ errmsg("%s requires a Boolean value, an integer "
+		"value greater than or equal to zero

Is there any reason why we follow this mixed style? 


Another nitpick in v6-0001 patch: 
tow whitespaces are between "special" and "value"
+		 second %s is the special  value "match" for that option */

Thanks,
Steven

view thread (5+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: file_fdw: Support multi-line HEADER option.
  In-Reply-To: <MN2PR15MB3021F257D141B286206F0DC6A796A@MN2PR15MB3021.namprd15.prod.outlook.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox