Re: Bug in pg_dump --filter? - Invalid object types can be misinterpreted as valid - Mailing list pgsql-hackers

On Sun, Aug 3, 2025 at 3:03 PM Xuneng Zhou <xunengzhou@gmail.com> wrote:
> After testing, the patch LGTM. I noticed two very small possible nits:

Thanks for the review!


> 1) Comment wording
>
> The loop now calls isspace((unsigned char)*ptr), so a token ends at
> any whitespace, not just at ASCII space (0x20). Could we revise the
> comment—from
> “strings of non-space characters bounded by space characters”
> to something like
> “strings of non-space characters bounded by whitespace”
> —to match the behavior?

I agree with the change. But the phrase "strings of non-space characters
bounded by whitespace" is a bit redundant, and "strings of non-whitespace
characters" is sufficient, isn't it? So I used that wording in the updated
patch I've attached.


> 2) Variable name
>
> const char *keyword = filter_get_token(&str, &size);
> keyword = filter_get_token(&str, &size);
>
> After the patch, filter_get_token() no longer returns a keyword
> (letters-only identifier); it now returns any non-whitespace token.
> Renaming the variable from keyword to token (or similar) might make
> the intent clearer..

This also got me thinking, if we simply define keywords as strings of
non-whitespace characters, maybe we don't need to change the term "keyword"
to "token" at all. I've updated the patch with that in mind. Thoughts?

Regards,

--
Fujii Masao