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

> Will do a closer review on the patch shortly.

Had a read through, and tested, the latest posted version today:

+    Read objects filters from the specified file. Specify "-" to read from
+    stdin. Lines of this file must have the following format:
I think this should be <filename>-</filename> and <literal>STDIN</literal> to
match the rest of the docs.


+       <para>
+        With the following filter file, the dump would include table
+        <literal>mytable1</literal> and data from foreign tables of
+        <literal>some_foreign_server</literal> foreign server, but exclude data
+        from table <literal>mytable2</literal>.
+<programlisting>
+include table mytable1
+include foreign_data some_foreign_server
+exclude table mytable2
+</programlisting>
+       </para>
This example is highlighting the issue I've previously raised with the UX/doc
of this feature.  The "exclude table mytable2" is totally pointless in the
above since the exact match of "mytable1" will remove all other objects.  What
we should be doing instead is use the pattern matching aspect along the lines
of the below:

    include table mytable*
    exclude table mytable2

+        The <option>--filter</option> option works just like the other
+        options to include or exclude tables, schemas, table data, or foreign
This should refer to the actual options by name to make it clear which we are
talking about.


+       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.


+    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.


+    initStringInfo(&line);
Why is this using a StringInfo rather than a PQExpBuffer as the rest of pg_dump
does?


+typedef struct
I think these should be at the top of the file with the other typedefs.


When testing strange object names, I was unable to express this name in the filter file:

$ ./bin/psql
psql (15devel)
Type "help" for help.

danielg=# create table "
danielg"# t
danielg"# t
danielg"# " (a integer);
CREATE TABLE
danielg=# select relname from pg_class order by oid desc limit 1;
 relname
---------
        +
 t      +
 t      +

(1 row)

--
Daniel Gustafsson        https://vmware.com/