Re: proposal: possibility to read dumped table's name from file - Mailing list pgsql-hackers

From Pavel Stehule
Subject Re: proposal: possibility to read dumped table's name from file
Date
Msg-id CAFj8pRAY8Bhvsj17v3ZyH9fGpPM_HkDK9LW+7Nnrs3K41kN2rQ@mail.gmail.com
Whole thread Raw
In response to Re: proposal: possibility to read dumped table's name from file  (Justin Pryzby <pryzby@telsasoft.com>)
List pgsql-hackers


po 13. 9. 2021 v 15:11 odesílatel Justin Pryzby <pryzby@telsasoft.com> napsal:
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".

I fixed (I hope) mentioned issues. Please check last patch

Regards

Pavel


--
Justin

pgsql-hackers by date:

Previous
From: Ranier Vilela
Date:
Subject: Re: Getting ERROR "subplan "SubPlan 1" was not initialized" in EXISTS subplan when using for list partition.
Next
From: Tom Lane
Date:
Subject: Re: Getting ERROR "subplan "SubPlan 1" was not initialized" in EXISTS subplan when using for list partition.