From: | Xuneng Zhou <xunengzhou(at)gmail(dot)com> |
---|---|
To: | Fujii Masao <masao(dot)fujii(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-05 00:14:37 |
Message-ID: | CABPTF7X5iLNuRKpou3epLncgBRV7WbmueSom18mxK2OdWpb30Q@mail.gmail.com |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Aug 4, 2025 at 11:18 PM Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>
> 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?
>
+1, this looks more elegant to me.
Best,
Xuneng
From | Date | Subject | |
---|---|---|---|
Next Message | Yugo Nagata | 2025-08-05 01:09:06 | Re: Doc: Add note for running ANALYZE after ALTER TABLE ... SET EXPRESSION |
Previous Message | Peter Smith | 2025-08-05 00:05:15 | Re: CREATE PUBLICATION with 'publish_generated_columns' parameter specified but unassigned |