From: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com> |
---|---|
To: | Xuneng Zhou <xunengzhou(at)gmail(dot)com> |
Cc: | srinath2133(at)gmail(dot)com, pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: Bug in pg_dump --filter? - Invalid object types can be misinterpreted as valid |
Date: | 2025-08-04 15:18:16 |
Message-ID: | CAHGQGwE4V68wh9CZbTbrGxjdguMsyA4pOO_0_LYLKRHVPDADuw@mail.gmail.com |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sun, Aug 3, 2025 at 3:03 PM Xuneng Zhou <xunengzhou(at)gmail(dot)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
Attachment | Content-Type | Size |
---|---|---|
v2-0001-pg_dump-Fix-incorrect-parsing-of-object-types-in-.patch | application/octet-stream | 4.0 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Fabrice Chapuis | 2025-08-04 15:23:08 | Re: set role issue |
Previous Message | Dean Rasheed | 2025-08-04 14:58:15 | Re: Improving and extending int128.h to more of numeric.c |