Re: proposal: possibility to read dumped table's name from file - Mailing list pgsql-hackers

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

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.

--
Justin