Hi
po 13. 9. 2021 v 15:01 odesílatel Daniel Gustafsson <dan...@yesql.se>
napsal:
> > On 28 Jul 2021, at 09:28, Pavel Stehule <pavel.steh...@gmail.com> wrote:
> > út 13. 7. 2021 v 1:16 odesílatel Tom Lane <t...@sss.pgh.pa.us <mailto:
> t...@sss.pgh.pa.us>> napsal:
>
> > Hence I suggest
> >
> > include table PATTERN
> > exclude table PATTERN
> >
> > which ends up being the above but with words not [+-].
>
> One issue with this syntax is that the include keyword can be quite
> misleading
> as it's semantic interpretion of "include table t" can be different from
> "--table=t". The former is less clear about the fact that it means
> "exclude
> all other tables than " then the latter. It can be solved with
> documentation,
> but I think that needs be to be made clearer.
>
I invite any documentation enhancing and fixing
>
> > Here is an updated implementation of filter's file, that implements
> syntax proposed by you.
>
> While it's not the format I would prefer, it does allow for most (all?) use
> cases expressed in this thread with ample armtwisting applied so let's go
> ahead
> from this point and see if we can agree on it (or a version of it).
>
> A few notes on the patch after a first pass over it:
>
> +(include|exclude)[table|schema|foreign_data|data] <replaceable
> class="parameter">objectname</replaceable>
> Lacks whitespace between keyword and object type. Also, since these are
> mandatory parameters, shouldn't they be within '{..}' ?
>
> yes, fixed
>
> + /* skip initial white spaces */
> + while (isblank(*ptr))
> + ptr += 1;
> We don't trust isblank() as of 3fd5faed5 due to portability concerns, this
> should probably use a version of the pg_isblank() we already have (and
> possibly
> move that to src/common/string.c as there now are more consumers).
>
>
I rewrote this part, and I don't use function isblank ever
>
> +static bool
> +isblank_line(const char *line)
> This could be replaced with a single call to strspn() as we already do for
> parsing the TOC file.
>
>
> + /* when first char is hash, ignore whole line */
> + if (*str == '#')
> + continue;
> I think we should strip leading whitespace before this to allow
> commentlines to
> start with whitespace, it's easy enough and will make life easier for
> users.
>
now, the comments can be used as first non blank char or after filter
>
>
> + pg_log_error("invalid format of filter file \"%s\": %s",
> + filename,
> + message);
> +
> + fprintf(stderr, "%d: %s\n", lineno, line);
> Can't we just include the lineno in the error logging and skip dumping the
> offending line? Fast-forwarding the pointer to print the offending part is
> less useful than a context marker, and in some cases suboptimal. With this
> coding, if a pattern is omitted for example the below error message is
> given:
>
>
pg_dump: error: invalid format of filter file "filter.txt": missing
> object name
> 1:
>
> The errormessage and the linenumber in the file should be enough for the
> user
> to figure out what to fix.
>
I did it like you proposed, but still, I think the content can be useful.
More times you read dynamically generated files, or you read data from
stdin, and in complex environments it can be hard regenerate new content
for debugging.
>
> + if (keyword && is_keyword(keyword, size, "table"))
> + objecttype = 't';
> Should this use an enum, or at least a struct translation the literal
> keyword
> to the internal representation? Magic constants without explicit
> connection to
> their token counterparts can easily be cause of bugs.
>
>
fixed
> If I create a table called "a\nb" and try to dump it I get an error in
> parsing
> the file. Isn't this supposed to work?
> $ cat filter.txt
> include table "a
> b"
> $ ./bin/pg_dump --filter=filter.txt
> pg_dump: error: invalid format of filter file "filter.txt": unexpected
> chars after object name
> 2:
probably there was some issue, because it should work. I tested a new
version and this is tested in new regress tests. Please, check
>
> Did you consider implementing this in Bison to abstract some of the messier
> parsing logic?
>
Initially not, but now, when I am thinking about it, I don't think so Bison
helps. The syntax of the filter file is nicely linear. Now, the code of the
parser is a little bit larger than minimalistic, but it is due to nicer
error's messages. The raw implementation in Bison raised just "syntax
error" and positions. I did code refactoring, and now the scanning, parsing
and processing are divided into separated routines. Parsing related code
has 90 lines. In this case, I don't think using a parser grammar file can
carry any benefit. grammar is more readable, sure, but we need to include
bison, we need to handle errors, and if we want to raise more helpful
errors than just "syntax error", then the code will be longer.
please, check attached patch
Regards
Pavel
> --
> Daniel Gustafsson https://vmware.com/
>
>
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index 7682226b99..1b74c0eadd 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -789,6 +789,55 @@ PostgreSQL documentation
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><option>--filter=<replaceable class="parameter">filename</replaceable></option></term>
+ <listitem>
+ <para>
+ Read objects filters from the specified file. Specify "-" to read from
+ stdin. Lines of this file must have the following format:
+<synopsis>
+{ include | exclude } { table | schema | foreign_data | data } <replaceable class="parameter">PATTERN</replaceable>
+</synopsis>
+ </para>
+
+ <para>
+ The first keyword specifies whether the object is to be included
+ or excluded, and the second keyword specifies the type of object
+ to be filtered:
+ <literal>table</literal> (table),
+ <literal>schema</literal> (schema),
+ <literal>foreign_data</literal> (foreign server),
+ <literal>data</literal> (table data).
+ </para>
+
+ <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>
+
+ <para>
+ Lines starting with <literal>#</literal> are ignored. The comment
+ (started by <literal>#</literal>) can be placed after filter too.
+ Empty lines are ignored too.
+ </para>
+
+ <para>
+ The <option>--filter</option> option works just like the other
+ options to include or exclude tables, schemas, table data, or foreign
+ tables, and both forms may be combined. Note that there are no options
+ to exclude a specific foreign table or to include a specific table's
+ data.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><option>--if-exists</option></term>
<listitem>
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index a485fb2d07..844492c64f 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -55,10 +55,12 @@
#include "catalog/pg_trigger_d.h"
#include "catalog/pg_type_d.h"
#include "common/connect.h"
+#include "common/string.h"
#include "dumputils.h"
#include "fe_utils/option_utils.h"
#include "fe_utils/string_utils.h"
#include "getopt_long.h"
+#include "lib/stringinfo.h"
#include "libpq/libpq-fs.h"
#include "parallel.h"
#include "pg_backup_db.h"
@@ -308,7 +310,7 @@ static void appendReloptionsArrayAH(PQExpBuffer buffer, const char *reloptions,
static char *get_synchronized_snapshot(Archive *fout);
static void setupDumpWorker(Archive *AHX);
static TableInfo *getRootTableInfo(const TableInfo *tbinfo);
-
+static void read_filters_from_file(char *filename, DumpOptions *dopt);
int
main(int argc, char **argv)
@@ -380,6 +382,7 @@ main(int argc, char **argv)
{"enable-row-security", no_argument, &dopt.enable_row_security, 1},
{"exclude-table-data", required_argument, NULL, 4},
{"extra-float-digits", required_argument, NULL, 8},
+ {"filter", required_argument, NULL, 12},
{"if-exists", no_argument, &dopt.if_exists, 1},
{"inserts", no_argument, NULL, 9},
{"lock-wait-timeout", required_argument, NULL, 2},
@@ -613,6 +616,10 @@ main(int argc, char **argv)
optarg);
break;
+ case 12: /* filter implementation */
+ read_filters_from_file(optarg, &dopt);
+ break;
+
default:
fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
exit_nicely(1);
@@ -1038,6 +1045,8 @@ help(const char *progname)
" access to)\n"));
printf(_(" --exclude-table-data=PATTERN do NOT dump data for the specified table(s)\n"));
printf(_(" --extra-float-digits=NUM override default setting for extra_float_digits\n"));
+ printf(_(" --filter=FILENAME dump objects and data based on the filter expressions\n"
+ " from the filter file\n"));
printf(_(" --if-exists use IF EXISTS when dropping objects\n"));
printf(_(" --include-foreign-data=PATTERN\n"
" include data of foreign tables on foreign\n"
@@ -18979,3 +18988,343 @@ appendReloptionsArrayAH(PQExpBuffer buffer, const char *reloptions,
if (!res)
pg_log_warning("could not parse reloptions array");
}
+
+typedef struct
+{
+ FILE *fp;
+ char *filename;
+ int lineno;
+} FilterStateData;
+
+typedef enum
+{
+ FILTER_OBJECT_TYPE_NONE,
+ FILTER_OBJECT_TYPE_TABLE,
+ FILTER_OBJECT_TYPE_SCHEMA,
+ FILTER_OBJECT_TYPE_FOREIGN_DATA,
+ FILTER_OBJECT_TYPE_DATA
+} FilterObjectType;
+
+/*
+ * Print error message and exit.
+ */
+static void
+exit_invalid_filter_format(FilterStateData *fstate, char *message)
+{
+ if (fstate->fp != stdin)
+ {
+ pg_log_error("invalid format of filter file \"%s\" on line %d: %s",
+ fstate->filename,
+ fstate->lineno,
+ message);
+
+ fclose(fstate->fp);
+ }
+ else
+ pg_log_error("invalid format of filter on line %d: %s",
+ fstate->lineno,
+ message);
+
+ exit_nicely(-1);
+}
+
+/*
+ * Search keyword (can contains only ascii alphabetic characters) on line.
+ * Returns NULL, when the line is empty or first char is not alpha
+ */
+static const char *
+filter_get_keyword(const char **line, int *size)
+{
+ const char *ptr = *line;
+ const char *result = NULL;
+
+ /* skip initial white spaces */
+ while (isspace(*ptr))
+ ptr += 1;
+
+ if (isascii(*ptr) && isalpha(*ptr))
+ {
+ result = ptr++;
+
+ while (isascii(*ptr) && (isalpha(*ptr) || *ptr == '_'))
+ ptr += 1;
+
+ *size = ptr - result;
+ }
+
+ *line = ptr;
+
+ return result;
+}
+
+static bool
+filter_is_keyword(const char *keyword, int size, const char *str)
+{
+ if (strlen(str) != size)
+ return false;
+
+ return pg_strncasecmp(keyword, str, size) == 0;
+}
+
+/*
+ * Sets objname to string with object identifier. The line variable holds string
+ * of last line with object identifier (object name). Returns pointer to first char
+ * after last char of object name.
+ */
+static char *
+filter_get_object_name(FilterStateData *fstate,
+ StringInfo line,
+ char *str,
+ char **objname)
+{
+ /* skip white spaces */
+ while (isspace(*str))
+ str++;
+
+ if (*str == '\0')
+ exit_invalid_filter_format(fstate, "missing object name");
+
+ if (*str == '"')
+ {
+ PQExpBuffer quoted_name = createPQExpBuffer();
+
+ appendPQExpBufferChar(quoted_name, '"');
+ str++;
+
+ while (1)
+ {
+ if (*str == '\0')
+ {
+ if (!pg_get_line_buf(fstate->fp, line))
+ {
+ if (ferror(fstate->fp))
+ fatal("could not read from file \"%s\": %m", fstate->filename);
+
+ exit_invalid_filter_format(fstate,"unexpected end of file");
+ }
+
+ (void) pg_strip_crlf(str);
+
+ appendPQExpBufferChar(quoted_name, '\n');
+ str = line->data;
+ fstate->lineno += 1;
+ }
+
+ if (*str == '"')
+ {
+ appendPQExpBufferChar(quoted_name, '"');
+ str++;
+
+ if (*str == '"')
+ str++;
+ else
+ break;
+ }
+ else if (*str == '\\')
+ {
+ str++;
+ if (*str == 'n')
+ appendPQExpBufferChar(quoted_name, '\n');
+ else if (*str == '\\')
+ appendPQExpBufferChar(quoted_name, '\\');
+
+ str++;
+ }
+ else
+ appendPQExpBufferChar(quoted_name, *str++);
+ }
+
+ *objname = quoted_name->data;
+ }
+ else
+ {
+ char *startptr = str++;
+
+ /* simple variant, read to end or to first space */
+ while (*str && !isspace(*str))
+ str++;
+
+ *objname = strndup(startptr, str - startptr);
+ }
+
+ return str;
+}
+
+/*
+ * Returns true, when one filter item was successfully read and parsed.
+ * When object name contains \n chars, then more than one line from input
+ * file can be processed. Returns false when EOF. Run exit on error.
+ */
+static bool
+read_filter_item(FilterStateData *fstate,
+ bool *is_include,
+ char **objname,
+ FilterObjectType *objtype)
+{
+ StringInfoData line;
+
+ initStringInfo(&line);
+
+ if (pg_get_line_buf(fstate->fp, &line))
+ {
+ char *str = line.data;
+ const char *keyword;
+ int size;
+
+ fstate->lineno += 1;
+
+ (void) pg_strip_crlf(str);
+
+ /* skip initial white spaces */
+ while (isspace(*str))
+ str++;
+
+ /*
+ * skip empty lines or lines when first noblank char is hash (comment)
+ */
+ if (*str != '\0' && *str != '#')
+ {
+ keyword = filter_get_keyword((const char **) &str, &size);
+ if (!keyword)
+ exit_invalid_filter_format(fstate,
+ "no keyword found (expected \"include\" or \"exclude\")");
+
+ /* Now we expect sequence of two keywords */
+ if (filter_is_keyword(keyword, size, "include"))
+ *is_include = true;
+ else if (filter_is_keyword(keyword, size, "exclude"))
+ *is_include = false;
+ else
+ exit_invalid_filter_format(fstate,
+ "invalid keyword (expected \"include\" or \"exclude\")");
+
+ keyword = filter_get_keyword((const char **) &str, &size);
+ if (!keyword)
+ exit_invalid_filter_format(fstate,
+ "no keyword found (expected \"table\", \"schema\", \"foreign_data\" or \"data\")");
+
+ if (filter_is_keyword(keyword, size, "table"))
+ *objtype = FILTER_OBJECT_TYPE_TABLE;
+ else if (filter_is_keyword(keyword, size, "schema"))
+ *objtype = FILTER_OBJECT_TYPE_SCHEMA;
+ else if (filter_is_keyword(keyword, size, "foreign_data"))
+ *objtype = FILTER_OBJECT_TYPE_FOREIGN_DATA;
+ else if (filter_is_keyword(keyword, size, "data"))
+ *objtype = FILTER_OBJECT_TYPE_DATA;
+ else
+ exit_invalid_filter_format(fstate,
+ "invalid keyword (expected \"table\", \"schema\", \"foreign_data\" or \"data\")");
+
+ str = filter_get_object_name(fstate, &line, str, objname);
+
+ /*
+ * check possible content after object identifier.
+ * Allow comment started by hash.
+ */
+ while (isspace(*str))
+ str++;
+
+ if (*str != '\0' && *str != '#')
+ exit_invalid_filter_format(fstate,
+ "unexpected chars after object name");
+ }
+ else
+ {
+ *objname = NULL;
+ *objtype = FILTER_OBJECT_TYPE_NONE;
+ }
+
+ free(line.data);
+
+ return true;
+ }
+
+ if (ferror(fstate->fp))
+ {
+ pg_log_fatal("could not read from file \"%s\": %m", fstate->filename);
+
+ if (fstate->fp != stdin)
+ fclose(fstate->fp);
+
+ exit_nicely(-1);
+ }
+
+ free(line.data);
+
+ return false;
+}
+
+/*
+ * Read dumped object specification from file
+ */
+static void
+read_filters_from_file(char *filename, DumpOptions *dopt)
+{
+ FilterStateData fstate;
+ bool is_include;
+ char *objname;
+ FilterObjectType objtype;
+
+ fstate.filename = filename;
+ fstate.lineno = 0;
+
+ /* use "-" as symbol for stdin */
+ if (strcmp(filename, "-") != 0)
+ {
+ fstate.fp = fopen(filename, "r");
+ if (!fstate.fp)
+ fatal("could not open the input file \"%s\": %m",
+ filename);
+ }
+ else
+ fstate.fp = stdin;
+
+ while (read_filter_item(&fstate, &is_include, &objname, &objtype))
+ {
+ if (objtype == FILTER_OBJECT_TYPE_TABLE)
+ {
+ if (is_include)
+ {
+ simple_string_list_append(&table_include_patterns, objname);
+ dopt->include_everything = false;
+ }
+ else
+ simple_string_list_append(&table_exclude_patterns, objname);
+ }
+ else if (objtype == FILTER_OBJECT_TYPE_SCHEMA)
+ {
+ if (is_include)
+ {
+ simple_string_list_append(&schema_include_patterns,
+ objname);
+ dopt->include_everything = false;
+ }
+ else
+ simple_string_list_append(&schema_exclude_patterns,
+ objname);
+ }
+ else if (objtype == FILTER_OBJECT_TYPE_DATA)
+ {
+ if (is_include)
+ exit_invalid_filter_format(&fstate,
+ "include filter is not allowed for this type of object");
+ else
+ simple_string_list_append(&tabledata_exclude_patterns,
+ objname);
+ }
+ else if (objtype == FILTER_OBJECT_TYPE_FOREIGN_DATA)
+ {
+ if (is_include)
+ simple_string_list_append(&foreign_servers_include_patterns,
+ objname);
+ else
+ exit_invalid_filter_format(&fstate,
+ "exclude filter is not allowed for this type of object");
+ }
+
+ free(objname);
+ }
+
+ if (fstate.fp != stdin)
+ fclose(fstate.fp);
+}
diff --git a/src/bin/pg_dump/t/004_pg_dump_filterfile.pl b/src/bin/pg_dump/t/004_pg_dump_filterfile.pl
new file mode 100644
index 0000000000..505614f145
--- /dev/null
+++ b/src/bin/pg_dump/t/004_pg_dump_filterfile.pl
@@ -0,0 +1,151 @@
+use strict;
+use warnings;
+
+use Config;
+use PostgresNode;
+use TestLib;
+use Test::More tests => 24;
+
+my $tempdir = TestLib::tempdir;
+my $inputfile;
+
+
+my $node = PostgresNode->new('main');
+my $port = $node->port;
+my $backupdir = $node->backup_dir;
+my $plainfile = "$backupdir/plain.sql";
+
+$node->init;
+$node->start;
+
+$node->safe_psql('postgres', "CREATE TABLE table_one(a varchar)");
+$node->safe_psql('postgres', "CREATE TABLE table_two(a varchar)");
+$node->safe_psql('postgres', "CREATE TABLE table_three(a varchar)");
+$node->safe_psql('postgres', "CREATE TABLE \"strange aaa
+name\"(a varchar)");
+$node->safe_psql('postgres', "INSERT INTO table_one VALUES('*** TABLE ONE ***')");
+$node->safe_psql('postgres', "INSERT INTO table_two VALUES('*** TABLE TWO ***')");
+$node->safe_psql('postgres', "INSERT INTO table_three VALUES('*** TABLE THREE ***')");
+
+open $inputfile, '>', "$tempdir/inputfile.txt";
+
+print $inputfile " include table table_one #comment\n";
+print $inputfile "include table table_two\n";
+print $inputfile "# skip this line\n";
+print $inputfile "\n";
+print $inputfile "exclude data table_one\n";
+close $inputfile;
+
+my ($cmd, $stdout, $stderr, $result);
+command_ok(
+ [ "pg_dump", '-p', $port, "-f", $plainfile, "--filter=$tempdir/inputfile.txt", 'postgres' ],
+ "dump tables with filter");
+
+my $dump = slurp_file($plainfile);
+
+ok($dump =~ qr/^CREATE TABLE public.table_one/m, "dumped table one");
+ok($dump =~ qr/^CREATE TABLE public.table_two/m, "dumped table two");
+ok($dump !~ qr/^CREATE TABLE public.table_three/m, "table three not dumped");
+ok($dump !~ qr/^COPY public.table_one/m, "content of table one is not included");
+ok($dump =~ qr/^COPY public.table_two/m, "content of table two is included");
+
+open $inputfile, '>', "$tempdir/inputfile.txt";
+
+print $inputfile "exclude table table_one\n";
+close $inputfile;
+
+command_ok(
+ [ "pg_dump", '-p', $port, "-f", $plainfile, "--filter=$tempdir/inputfile.txt", 'postgres' ],
+ "dump tables with filter");
+
+$dump = slurp_file($plainfile);
+
+ok($dump !~ qr/^CREATE TABLE public.table_one/m, "table one not dumped");
+ok($dump =~ qr/^CREATE TABLE public.table_two/m, "dumped table two");
+ok($dump =~ qr/^CREATE TABLE public.table_three/m, "dumped table three");
+
+open $inputfile, '>', "$tempdir/inputfile.txt";
+
+print $inputfile "include table \"strange aaa
+name\"";
+close $inputfile;
+
+command_ok(
+ [ "pg_dump", '-p', $port, "-f", $plainfile, "--filter=$tempdir/inputfile.txt", 'postgres' ],
+ "dump tables with filter");
+
+$dump = slurp_file($plainfile);
+
+ok($dump =~ qr/^CREATE TABLE public.\"strange aaa/m, "dump table with new line in name");
+
+
+
+open $inputfile, '>', "$tempdir/inputfile.txt";
+
+print $inputfile "exclude table \"strange aaa\\nname\"";
+close $inputfile;
+
+command_ok(
+ [ "pg_dump", '-p', $port, "-f", $plainfile, "--filter=$tempdir/inputfile.txt", 'postgres' ],
+ "dump tables with filter");
+
+$dump = slurp_file($plainfile);
+
+ok($dump !~ qr/^CREATE TABLE public.\"strange aaa/m, "dump table with new line in name");
+
+open $inputfile, '>', "$tempdir/inputfile.txt";
+
+print $inputfile "exclude schema public\n";
+close $inputfile;
+
+command_ok(
+ [ "pg_dump", '-p', $port, "-f", $plainfile, "--filter=$tempdir/inputfile.txt", 'postgres' ],
+ "dump tables with filter");
+
+$dump = slurp_file($plainfile);
+
+ok($dump !~ qr/^CREATE TABLE/m, "no table dumped");
+
+#########################################
+# For test of +f option we need created foreign server or accept
+# fail and check error
+
+open $inputfile, '>', "$tempdir/inputfile.txt";
+
+print $inputfile "include foreign_data doesnt_exists\n";
+close $inputfile;
+
+command_fails_like(
+ [ "pg_dump", '-p', $port, "-f", $plainfile, "--filter=$tempdir/inputfile.txt", 'postgres' ],
+ qr/pg_dump: error: no matching foreign servers were found for pattern/,
+ "dump foreign server");
+
+#########################################
+# Test broken input format
+
+open $inputfile, '>', "$tempdir/inputfile.txt";
+print $inputfile "k";
+close $inputfile;
+
+command_fails_like(
+ [ "pg_dump", '-p', $port, "-f", $plainfile, "--filter=$tempdir/inputfile.txt", 'postgres' ],
+ qr/invalid keyword/,
+ "broken format check");
+
+open $inputfile, '>', "$tempdir/inputfile.txt";
+print $inputfile "include xxx";
+close $inputfile;
+
+command_fails_like(
+ [ "pg_dump", '-p', $port, "-f", $plainfile, "--filter=$tempdir/inputfile.txt", 'postgres' ],
+ qr/invalid keyword/,
+ "broken format check");
+
+open $inputfile, '>', "$tempdir/inputfile.txt";
+print $inputfile "include table";
+close $inputfile;
+
+command_fails_like(
+ [ "pg_dump", '-p', $port, "-f", $plainfile, "--filter=$tempdir/inputfile.txt", 'postgres' ],
+ qr/missing object name/,
+ "broken format check");
