Re: proposal: possibility to read dumped table's name from file

ne 19. 3. 2023 v 15:01 odesílatel Justin Pryzby <pryzby@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" ?
>

changed


>
> 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.
>

fixed


>
> > +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.
>

should be fixed by you


>
> > + * 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?
>

It is usual behaviour in Postgres for keywords. Important is the complete
sentence "Returns NULL when the buffer is empty or the first char is not
alpha."

In this case this implementation has no big impact on behaviour - probably
you got a message "unknown keyword" instead of "missing keyword". But I
would
implement behaviour consistent with other places. My opinion in this case
is not extra strong - we can define the form of keywords like we want, just
this is consistent
with other parsers in Postgres.



>
> > +                             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");
>

done



>
> That makes shorter and fewer strings.
>
> Find attached a bunch of other corrections as 0002.txt
>

merged

Regards

Pavel