Re: file_fdw: Support multi-line HEADER option. - Mailing list pgsql-hackers

> On Jan 15, 2026, at 19:44, Shinya Kato <shinya11.kato@gmail.com> wrote:
>
> On Mon, Jan 12, 2026 at 1:01 PM Chao Li <li.evan.chao@gmail.com> wrote:
>> Thanks for the patch. Here are a few review comments:
>
> Thank you for the review!
>
>> 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?
>
> The header option ends up as an integer line count in
> defGetCopyHeaderOption whether the value is quoted or not, so we don't
> need to distinguish between them. But as you said, it is ambiguous, so
> I updated the comment and added a test case.
>

I am sorry maybe I didn’t express myself clear. But in v4, this problem is clearer:

1 - 0001
```
     /*
-     * Allow 0, 1, "true", "false", "on", "off", a non-negative integer, or
-     * "match".
+     * Allow 0, 1, "true", "false", "on", "off", an integer greater than or
+     * equal to zero, or "match".
      */
```

Here, “0, 1” is a duplicate of “an integer greater than or equal to zero”, so the commend can be simplified as:

```
Allow “true”, “false”, “on”, “off”, an integer greater than or equal to zero, or ...
```

And one more comment for 0002:

2 - 0002

Looking at the two error branches:

```
+                else
+                {
+                    ErrorSaveContext escontext = {T_ErrorSaveContext};
+
+                    /* Check if the header is a valid integer */
+                    ival = pg_strtoint32_safe(sval, (Node *) &escontext);
+                    if (escontext.error_occurred)
+                        ereport(ERROR,
+                                (errcode(ERRCODE_SYNTAX_ERROR),
+                                 errmsg("%s requires a Boolean value, an integer greater than or "
+                                        "equal to zero, or the string \"match\"",
+                                        def->defname)));
+                }
```
and
```
+    if (ival < 0)
+        ereport(ERROR,
+                (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                 errmsg("a negative integer value cannot be "
+                        "specified for %s", def->defname)));
```

For the "ival<0" case, I think we can use the same error message as the first one, because the error message “an
integergreater than or equal to zero” has covered the error of “ival<0”. It would be better to generate less different
errormessages. 

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/