On Thu, Oct 13, 2022 at 11:46:34AM -0700, Andres Freund wrote: > Hi, > > On 2022-10-07 07:26:08 +0200, Pavel Stehule wrote: > > I am sending version with handy written parser and meson support > > Given this is a new approach it seems inaccurate to have the CF entry marked > ready-for-committer. I've updated it to needs-review.
I just had a quick look at the rest of the patch. For the parser, it seems that filter_get_pattern is reimplementing an identifier parsing function but isn't entirely correct. It can correctly parse quoted non-qualified identifiers and non-quoted qualified identifiers, but not quoted and qualified ones. For instance: $ echo 'include table nsp.tbl' | pg_dump --filter - >/dev/null $echo $? 0 $ echo 'include table "TBL"' | pg_dump --filter - >/dev/null $echo $? 0 $ echo 'include table "NSP"."TBL"' | pg_dump --filter - >/dev/null pg_dump: error: invalid format of filter on line 1: unexpected extra data after pattern This should also be covered in the regression tests. I'm wondering if psql's parse_identifier() could be exported and reused here rather than creating yet another version. Nitpicking: the comments needs some improvements: + /* + * Simple routines - just don't repeat same code + * + * Returns true, when filter's file is opened + */ + bool + filter_init(FilterStateData *fstate, const char *filename) also, is there any reason why this function doesn't call exit_nicely in case of error rather than letting each caller do it without any other cleanup? + /* + * Release allocated sources for filter + */ + void + filter_free_sources(FilterStateData *fstate) I'm assuming "ressources" not "sources"? + /* + * log_format_error - Emit error message + * + * This is mostly a convenience routine to avoid duplicating file closing code + * in multiple callsites. + */ + void + log_invalid_filter_format(FilterStateData *fstate, char *message) mismatch between comment and function name (same for filter_read_item) + static const char * + filter_object_type_name(FilterObjectType fot) No description. /* * Helper routine to reduce duplicated code */ void log_unsupported_filter_object_type(FilterStateData *fstate, const char *appname, FilterObjectType fot) Need more helpful comment.