Date: Sun, 17 Jul 2022 09:01:46 -0500
From: Justin Pryzby <pryzby@telsasoft.com>
To: Pavel Stehule <pavel.stehule@gmail.com>
Cc: Andrew Dunstan <andrew@dunslane.net>,
	Daniel Gustafsson <daniel@yesql.se>, Tom Lane <tgl@sss.pgh.pa.us>,
	Alvaro Herrera <alvherre@2ndquadrant.com>,
	Tomas Vondra <tomas.vondra@enterprisedb.com>,
	Dean Rasheed <dean.a.rasheed@gmail.com>,
	Stephen Frost <sfrost@snowman.net>,
	Surafel Temesgen <surafel3000@gmail.com>,
	vignesh C <vignesh21@gmail.com>, pgsql-hackers@postgresql.org
Subject: Re: proposal: possibility to read dumped table's name from file
Message-ID: <20220717140146.GY18011@telsasoft.com>
References: <202107122259.n6o5uwb5erza@alvherre.pgsql>
 <3183720.1626131795@sss.pgh.pa.us>
 <CAFj8pRBq9X=oSNbo+v93EwznkQ3JczEW6UsfQK37vx1ktW-2ZA@mail.gmail.com>
 <B854A710-E1DE-4C6B-A411-E246A95DCDF9@yesql.se>
 <CAFj8pRAZbCvM3v8jp5wkPpZHJvAopWpbF=+v+84Pns=qWtvEcg@mail.gmail.com>
 <CAFj8pRAy+xfSj=tz_=RRx0hsn_x5dMpvTz2RJ9OzCG9GTdPxnA@mail.gmail.com>
 <CAFj8pRCk0Syge+c1mvkHwpJ6S2zM7nyEMu98iyzRWYsccXuG+g@mail.gmail.com>
 <7f9cde55-dd3b-a604-bf89-e70511025709@dunslane.net>
 <CAFj8pRBJsxuT8DK0UZqS07zPA6dgRcU72=5ymJEUpWUVej0wNQ@mail.gmail.com>
 <CAFj8pRDQmz86g03emFHqjHejB+gG_4KqrLij=L1L6Z-59dzL0g@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CAFj8pRDQmz86g03emFHqjHejB+gG_4KqrLij=L1L6Z-59dzL0g@mail.gmail.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Archived-At: 
 <https://www.postgresql.org/message-id/20220717140146.GY18011%40telsasoft.com>
Precedence: bulk

Thanks for updating the patch.

This failed to build on windows.
http://cfbot.cputube.org/pavel-stehule.html

Some more comments inline.

On Sun, Jul 17, 2022 at 08:20:47AM +0200, Pavel Stehule wrote:
> The attached patch implements the --filter option for pg_dumpall and for
> pg_restore too.

> diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
> index 5efb442b44..ba2920dbee 100644
> --- a/doc/src/sgml/ref/pg_dump.sgml
> +++ b/doc/src/sgml/ref/pg_dump.sgml
> @@ -779,6 +779,80 @@ PostgreSQL documentation
>        </listitem>
>       </varlistentry>
>  
> +     <varlistentry>
> +      <term><option>--filter=<replaceable class="parameter">filename</replaceable></option></term>
> +      <listitem>
> +       <para>
> +        Specify a filename from which to read patterns for objects to include
> +        or exclude from the dump. The patterns are interpreted according to the
> +        same rules as the corresponding options:
> +        <option>-t</option>/<option>--table</option> for tables,
> +        <option>-n</option>/<option>--schema</option> for schemas,
> +        <option>--include-foreign-data</option> for data on foreign servers and
> +        <option>--exclude-table-data</option> for table data.
> +        To read from <literal>STDIN</literal> use <filename>-</filename> as the

STDIN comma

> +       <para>
> +        Lines starting with <literal>#</literal> are considered comments and
> +        are ignored. Comments can be placed after filter as well. Blank lines

change "are ignored" to "ignored", I think.

> diff --git a/doc/src/sgml/ref/pg_dumpall.sgml b/doc/src/sgml/ref/pg_dumpall.sgml
> index 8a081f0080..137491340c 100644
> --- a/doc/src/sgml/ref/pg_dumpall.sgml
> +++ b/doc/src/sgml/ref/pg_dumpall.sgml
> @@ -122,6 +122,29 @@ PostgreSQL documentation
>        </listitem>
>       </varlistentry>
>  
> +     <varlistentry>
> +      <term><option>--filter=<replaceable class="parameter">filename</replaceable></option></term>
> +      <listitem>
> +       <para>
> +        Specify a filename from which to read patterns for databases excluded
> +        from dump. The patterns are interpretted according to the same rules
> +        like <option>--exclude-database</option>.

same rules *as*

> +        To read from <literal>STDIN</literal> use <filename>-</filename> as the

comma

> +        filename.  The <option>--filter</option> option can be specified in
> +        conjunction with the above listed options for including or excluding

For dumpall, remove "for including or"
change "above listed options" to "exclude-database" ?

> diff --git a/doc/src/sgml/ref/pg_restore.sgml b/doc/src/sgml/ref/pg_restore.sgml
> index 526986eadb..5f16c4a333 100644
> --- a/doc/src/sgml/ref/pg_restore.sgml
> +++ b/doc/src/sgml/ref/pg_restore.sgml
> @@ -188,6 +188,31 @@ PostgreSQL documentation
>        </listitem>
>       </varlistentry>
>  
> +     <varlistentry>
> +      <term><option>--filter=<replaceable class="parameter">filename</replaceable></option></term>
> +      <listitem>
> +       <para>
> +        Specify a filename from which to read patterns for objects excluded
> +        or included from restore. The patterns are interpretted according to the
> +        same rules like <option>--schema</option>, <option>--exclude-schema</option>,

s/like/as/

> +        <option>--function</option>, <option>--index</option>, <option>--table</option>
> +        or <option>--trigger</option>.
> +        To read from <literal>STDIN</literal> use <filename>-</filename> as the

STDIN comma

> +/*
> + * filter_get_keyword - read the next filter keyword from buffer
> + *
> + * Search for keywords (limited to containing ascii alphabetic characters) in

remove "containing"

> +	/*
> +	 * If the object name pattern has been quoted we must take care parse out
> +	 * the entire quoted pattern, which may contain whitespace and can span
> +	 * over many lines.

quoted comma
*to parse
remove "over"

> + * The pattern is either simple without any  whitespace, or properly quoted

double space

> + * in case there is whitespace in the object name. The pattern handling follows

s/is/may be/

> +			if (size == 7 && pg_strncasecmp(keyword, "include", 7) == 0)
> +				*is_include = true;
> +			else if (size == 7 && pg_strncasecmp(keyword, "exclude", 7) == 0)
> +				*is_include = false;

Can't you write strncasecmp(keyword, "include", size) to avoid hardcoding "7" ?

> +
> +			if (size == 4 && pg_strncasecmp(keyword, "data", 4) == 0)
> +				*objtype = FILTER_OBJECT_TYPE_DATA;
> +			else if (size == 8 && pg_strncasecmp(keyword, "database", 8) == 0)
> +				*objtype = FILTER_OBJECT_TYPE_DATABASE;
> +			else if (size == 12 && pg_strncasecmp(keyword, "foreign_data", 12) == 0)
> +				*objtype = FILTER_OBJECT_TYPE_FOREIGN_DATA;
> +			else if (size == 8 && pg_strncasecmp(keyword, "function", 8) == 0)
> +				*objtype = FILTER_OBJECT_TYPE_FUNCTION;
> +			else if (size == 5 && pg_strncasecmp(keyword, "index", 5) == 0)
> +				*objtype = FILTER_OBJECT_TYPE_INDEX;
> +			else if (size == 6 && pg_strncasecmp(keyword, "schema", 6) == 0)
> +				*objtype = FILTER_OBJECT_TYPE_SCHEMA;
> +			else if (size == 5 && pg_strncasecmp(keyword, "table", 5) == 0)
> +				*objtype = FILTER_OBJECT_TYPE_TABLE;
> +			else if (size == 7 && pg_strncasecmp(keyword, "trigger", 7) == 0)
> +				*objtype = FILTER_OBJECT_TYPE_TRIGGER;

Avoid hardcoding these constants.

> diff --git a/src/bin/pg_dump/filter.h b/src/bin/pg_dump/filter.h
> new file mode 100644
> index 0000000000..e4a1a74b10
> --- /dev/null
> +++ b/src/bin/pg_dump/filter.h
...
> \ No newline at end of file

:(