> On Jan 15, 2026, at 19:44, Shinya Kato <[email protected]> wrote:
>
> On Mon, Jan 12, 2026 at 1:01 PM Chao Li <[email protected]> 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 integer greater than or equal to zero” has
covered the error of “ival<0”. It would be better to generate less different
error messages.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
