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

Поиск
Список
Период
Сортировка
От Pavel Stehule
Тема Re: proposal: possibility to read dumped table's name from file
Дата
Msg-id CAFj8pRDN8_jqzk8SppfYLeXw02XeNf_zYO28ZjjPf1hcQFe30g@mail.gmail.com
обсуждение исходный текст
Ответ на Re: proposal: possibility to read dumped table's name from file  (Justin Pryzby <pryzby@telsasoft.com>)
Ответы Re: proposal: possibility to read dumped table's name from file  (Justin Pryzby <pryzby@telsasoft.com>)
Список pgsql-hackers


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
 
Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Michael Paquier
Дата:
Сообщение: Missing rules for queryjumblefuncs.{funcs,switch}.c for maintainer-clean
Следующее
От: Richard Guo
Дата:
Сообщение: Re: Missing rules for queryjumblefuncs.{funcs,switch}.c for maintainer-clean