On Sun, Aug 3, 2025 at 3:03 PM Xuneng Zhou <xunengz...@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
v2-0001-pg_dump-Fix-incorrect-parsing-of-object-types-in-.patch
Description: Binary data
