| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Shinya Kato <shinya11(dot)kato(at)gmail(dot)com> |
| Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> |
| Subject: | Re: file_fdw: Support multi-line HEADER option. |
| Date: | 2026-01-12 04:01:15 |
| Message-ID: | A1B370D0-4A29-487F-8E52-3120C717A8DA@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On Jan 9, 2026, at 16:14, Shinya Kato <shinya11(dot)kato(at)gmail(dot)com> wrote:
>
> Hi hackers,
> (CC: Fujii-san, committer of bc2f348e8)
>
> Commit bc2f348e8[0] introduced multi-line HEADER support for COPY.
> However, file_fdw does not yet support it, so I have implemented it in
> the attached patch.
>
> Since foreign table options in CREATE/ALTER FOREIGN TABLE are
> specified as single-quoted strings, I updated defGetCopyHeaderOption()
> to handle string values as well.
>
> Thoughts?
>
> [0] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=bc2f348e87c02de63647dbe290d64ff088880dbe
>
> --
> Best regards,
> Shinya Kato
> NTT OSS Center
> <v1-0001-file_fdw-Support-multi-line-HEADER-option.patch>
Hi Shinya,
Thanks for the patch. Here are a few review comments:
1
```
- * Allow 0, 1, "true", "false", "on", "off", a non-negative integer, or
- * "match".
+ * Allow 0, 1, "true", "false", "on", "off", a non-negative integer (also
+ * as a string, to support file_fdw options), or "match".
*/
```
From this comment, I cannot get how “0” and “1” will behave, and I cannot find a test case to show me that.
With this patch, “2” acts the same as 2, so “1” acts the same as 1. Will “1” be a line count or a boolean true?
2
```
+ /* Check if the header is a valid integer */
+ ival = pg_strtoint32_safe(header, (Node *)&escontext);
+ if (escontext.error_occurred)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("%s requires a Boolean value, a non-negative integer, "
+ "or the string \"match\"",
+ def->defname)));
```
I am thinking if INVALID_PARAMETER_VALUE would be better fit here for the error code.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | David G. Johnston | 2026-01-12 04:04:28 | Re: docs: clarify ALTER TABLE behavior on partitioned tables |
| Previous Message | Chao Li | 2026-01-12 03:30:58 | Re: pg_upgrade: optimize replication slot caught-up check |