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 CAFj8pRBgWfp6wQVmkqj5FWivSPqQaLH916uYZ1yomruRtrxQ8w@mail.gmail.com
обсуждение исходный текст
Ответ на Re: proposal: possibility to read dumped table's name from file  (Daniel Gustafsson <daniel@yesql.se>)
Список pgsql-hackers


út 21. 9. 2021 v 14:37 odesílatel Daniel Gustafsson <daniel@yesql.se> napsal:
> On 21 Sep 2021, at 08:50, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> po 20. 9. 2021 v 14:10 odesílatel Daniel Gustafsson <daniel@yesql.se <mailto:daniel@yesql.se>> napsal:

> +       printf(_("  --filter=FILENAME            dump objects and data based on the filter expressions\n"
> +                        "                               from the filter file\n"));
> Before we settle on --filter I think we need to conclude whether this file is
> intended to be included from a config file, or used on it's own.  If we gow tih
> the former then we might not want a separate option for just --filter.
>
> I prefer to separate two files. Although there is some intersection, I think it is good to have two simple separate files for two really different tasks.
> It does filtering, and it should be controlled by option "--filter". When the implementation will be changed, then this option can be changed too.
> Filtering is just a pg_dump related feature. Revision of client application configuration is a much more generic task, and if we mix it to one, we can be
> in a trap. It can be hard to find one good format for large script generated content, and possibly hand written structured content. For practical
> reasons it can be good to have two files too. Filters and configurations can have different life cycles.

I'm not convinced that we can/should change or remove a commandline parameter
in a coming version when there might be scripts expecting it to work in a
specific way.  Having a --filter as well as a --config where the configfile can
refer to the filterfile also passed via --filter sounds like problem waiting to
happen, so I think we need to settle how we want to interact with this file
before anything goes in.

Any thoughts from those in the thread who have had strong opinions on config
files etc?

> +    if (filter_is_keyword(keyword, size, "include"))
> I would prefer if this function call was replaced by just the pg_strcasecmp()
> call in filter_is_keyword() and the strlen optimization there removed.  The is
> not a hot-path, we can afford the string comparison in case of errors.  Having
> the string comparison done inline here will improve readability saving the
> reading from jumping to another function to see what it does.
>
> I agree that this is not a hot-path, just I don't feel well if I need to make a zero end string just for comparison pg_strcasecmp. Current design reduces malloc/free cycles. It is used in more places, when Postgres parses strings - SQL parser, plpgsql parser. I am not sure about the benefits and costs - pg_strcasecmp can be more readable, but for any keyword I have to call pstrdup and pfree. Is it necessary? My opinion in this part is not too strong - it is a minor issue, maybe I have a little bit different feelings about benefits and costs in this specific case, and if you really think the benefits of rewriting are higher, I'll do it

Sorry, I typoed my response.  What I meant was to move the pg_strncasecmp call
inline and not do the strlen check, to save readers from jumping around.  So
basically end up with the below in read_filter_item():

+       /* Now we expect sequence of two keywords */
+       if (pg_strncasecmp(keyword, "include", size) == 0)
+               *is_include = true;


I don't think so it is safe (strict). Only pg_strncasecmp(..)  is true for keywords "includex", "includedsss", ... You should to compare the size

Regards

Pavel
 

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

Предыдущее
От: torikoshia
Дата:
Сообщение: Re: EXPLAIN(VERBOSE) to CTE with SEARCH BREADTH FIRST fails
Следующее
От: Jeevan Ladhe
Дата:
Сообщение: Re: refactoring basebackup.c