ne 19. 3. 2023 v 15:01 odesÃlatel Justin Pryzby <pry...@telsasoft.com> napsal:
> On Thu, Mar 16, 2023 at 01:05:41PM +0100, Pavel Stehule wrote: > > rebase + enhancing about related option from a563c24 > > Thanks. > > It looks like this doesn't currently handle extensions, which were added > at 6568cef26e. > > > + <literal>table_and_children</literal>: tables, works like > > + <option>-t</option>/<option>--table</option>, except that > > + it also includes any partitions or inheritance child > > + tables of the table(s) matching the > > + <replaceable class="parameter">pattern</replaceable>. > > Why doesn't this just say "works like --table-and-children" ? > > I think as you wrote log_invalid_filter_format(), the messages wouldn't > be translated, because they're printed via %s. One option is to call > _() on the message. > > > +ok($dump !=~ qr/^CREATE TABLE public\.bootab/m, "exclude dumped > children table"); > > !=~ is being interpretted as as numeric "!=" and throwing warnings. > It should be a !~ b, right ? > It'd be nice if perl warnings during the tests were less easy to miss. > > > + * char is not alpha. The char '_' is allowed too (exclude first > position). > > Why is it treated specially? Could it be treated the same as alpha? > > > + log_invalid_filter_format(&fstate, > > + > "\"include\" table data filter is not allowed"); > > + log_invalid_filter_format(&fstate, > > + > "\"include\" table data and children filter is not allowed"); > > For these, it might be better to write the literal option: > > > + > "include filter for \"table_data_and_children\" is not allowed"); > > Because the option is a literal and shouldn't be translated. > And it's probably better to write that using %s, like: > > > + > "include filter for \"%s\" is not allowed"); > > That makes shorter and fewer strings. > > Find attached a bunch of other corrections as 0002.txt > Thank you very much - I'll recheck the mentioned points tomorrow. > > I also dug up what I'd started in november, trying to reduce the code > duplication betwen pg_restore/dump/all. This isn't done, but I might > never finish it, so I'll at least show what I have in case you think > it's a good idea. This passes tests on CI, except for autoconf, due to > using exit_nicely() differently. > Your implementation reduced 60 lines, but the interface and code is more complex. I cannot say what is significantly better. Personally, in this case, I prefer my variant, because I think it is a little bit more readable, and possible modification can be more simple. But this is just my opinion, and I have no problem accepting other opinions. I can imagine to define some configuration array like getopt, but it looks like over engineering Regards Pavel > -- > Justin >
