po 13. 11. 2023 v 14:39 odesílatel Daniel Gustafsson <dan...@yesql.se> napsal:
> > On 13 Nov 2023, at 14:15, Pavel Stehule <pavel.steh...@gmail.com> wrote: > > > > Hi > > > > ne 12. 11. 2023 v 14:17 odesílatel Pavel Stehule < > pavel.steh...@gmail.com <mailto:pavel.steh...@gmail.com>> napsal: > > Hi > > > > > > What are your thoughts on this version? It's not in a committable state > as it > > needs a bit more comments here and there and a triplecheck that nothing > was > > missed in changing this, but I prefer to get your thoughts before > spending the > > extra time. > > > > I think using pointer to exit function is an elegant solution. I checked > the code and I found only one issue. I fixed warning > > > > [13:57:22.578] time make -s -j${BUILD_JOBS} world-bin > > [13:58:20.858] filter.c: In function ‘pg_log_filter_error’: > > [13:58:20.858] filter.c:161:2: error: function ‘pg_log_filter_error’ > might be a candidate for ‘gnu_printf’ format attribute > [-Werror=suggest-attribute=format] > > [13:58:20.858] 161 | vsnprintf(buf, sizeof(buf), fmt, argp); > > [13:58:20.858] | ^~~~~~~~~ > > [13:58:20.858] cc1: all warnings being treated as errors > > > > and probably copy/paste bug > > > > diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c > > index f647bde28d..ab2abedf5f 100644 > > --- a/src/bin/pg_dump/pg_restore.c > > +++ b/src/bin/pg_dump/pg_restore.c > > @@ -535,7 +535,7 @@ read_restore_filters(const char *filename, > RestoreOptions *opts) > > case FILTER_OBJECT_TYPE_EXTENSION: > > case FILTER_OBJECT_TYPE_FOREIGN_DATA: > > pg_log_filter_error(&fstate, _("%s filter for \"%s\" > is not allowed."), > > - "exclude", > > + "include", > > > filter_object_type_name(objtype)); > > exit_nicely(1); > > > > Regards > > > > Pavel > > > > next update - fix used, but uninitialized "is_include" variable, when > filter is of FILTER_OBJECT_TYPE_NONE > > Thanks, the posted patchset was indeed a bit of a sketch, thanks for > fixing up > these. I'll go over it again too to clean it up and try to make into > something > committable. > > I was pondering replacing the is_include handling with returning an enum > for > the operation, to keep things more future proof in case we add more > operations > (and also a bit less magic IMHO). > +1 Pavel > -- > Daniel Gustafsson > >
