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

On Wed, Jul 28, 2021 at 09:28:17AM +0200, Pavel Stehule wrote:
> Here is an updated implementation of filter's file, that implements syntax
> proposed by you.

Thanks.

If there's any traction for this approach. I have some comments for the next
revision,

> +++ b/doc/src/sgml/ref/pg_dump.sgml
> @@ -789,6 +789,56 @@ PostgreSQL documentation
> </listitem>
> </varlistentry>
>
> + <varlistentry>
> + <term><option>--filter=<replaceable class="parameter">filename</replaceable></option></term>
> + <listitem>
> + <para>
> + Read objects filters from the specified file.
> + If you use "-" as a filename, the filters are read from stdin.

Say 'Specify "-" to read from stdin'

> + The lines starting with symbol <literal>#</literal> are ignored.

Remove "The" and "symbol"

> + Previous white chars (spaces, tabs) are not allowed. These

Preceding whitespace characters...

But actually, they are allowed? But if it needs to be explained, maybe they
shouldn't be - I don't see the utility of it.

> +static bool
> +isblank_line(const char *line)
> +{
> + while (*line)
> + {
> + if (!isblank(*line++))
> + return false;
> + }
> +
> + return true;
> +}

I don't think this requires nor justifies having a separate function.
Either don't support blank lines, or use get_keyword() with size==0 for that ?

> + /* Now we expect sequence of two keywords */
> + if (keyword && is_keyword(keyword, size, "include"))
> + is_include = true;
> + else if (keyword && is_keyword(keyword, size, "exclude"))
> + is_include = false;
> + else

I think this should first check "if keyword == NULL".
That could give a more specific error message like "no keyword found",

> + exit_invalid_filter_format(fp,
> + filename,
> + "expected keyword \"include\" or \"exclude\"",
> + line.data,
> + lineno);

..and then this one can say "invalid keyword".

--
Justin