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
