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

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

In response to

Browse pgsql-hackers by date

  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