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