st 10. 6. 2020 v 0:30 odesÃlatel Justin Pryzby <pry...@telsasoft.com>
napsal:
> On Tue, Jun 09, 2020 at 11:46:24AM +0200, Pavel Stehule wrote:
> > po 8. 6. 2020 v 23:30 odesÃlatel Justin Pryzby <pry...@telsasoft.com>
> napsal:
>
> > I still wonder if a better syntax would use a unified --filter option,
> whose
> > > argument would allow including/excluding any type of object:
> > I tried to implement simple format "[+-][tndf] objectname"
>
> Thanks.
>
> > + /* ignore empty rows */
> > + if (*line != '\0')
>
> Maybe: if line=='\0': continue
>
ok
> We should also support comments.
>
> > + bool
> include_filter = false;
> > + bool
> exclude_filter = false;
>
> I think we only need one bool.
> You could call it: bool is_exclude = false
>
>
ok
> +
> > + if (chars < 4)
> > +
> invalid_filter_format(optarg, line, lineno);
>
> I think that check is too lax.
> I think it's ok if we require the first char to be [-+] and the 2nd char
> to be
> [dntf]
>
> > + objecttype =
> line[1];
>
> ... but I think this is inadequately "liberal in what it accepts"; I think
> it
> should skip spaces. In my proposed scheme, someone might reasonably write:
>
> > +
> > + objectname =
> &line[3];
> > +
> > + /* skip initial
> spaces */
> > + while (*objectname
> == ' ')
> > +
> objectname++;
>
> I suggest to use isspace()
>
ok
> I think we should check that *objectname != '\0', rather than chars>=4,
> above.
>
done
> > + if
> (include_filter)
> > + {
> > +
> simple_string_list_append(&table_include_patterns, objectname);
> > +
> dopt.include_everything = false;
> > + }
> > + else if
> (exclude_filter)
> > +
> simple_string_list_append(&table_exclude_patterns, objectname);
>
> If you use bool is_exclude, then this becomes "else" and you don't need to
> think about checking if (!include && !exclude).
>
> > + else if
> (objecttype == 'f')
> > + {
> > + if
> (include_filter)
> > +
> simple_string_list_append(&foreign_servers_include_patterns, objectname);
> > + else if
> (exclude_filter)
> > +
> invalid_filter_format(optarg, line, lineno);
> > + }
>
> I would handle invalid object types as "else: invalid_filter_format()"
> here,
> rather than duplicating above as: !=ALL('d','n','t','f')
>
good idea
> > +
> > + if (ferror(f))
> > + fatal("could not read from
> file \"%s\": %s",
> > + f == stdin ?
> "stdin" : optarg,
> > + strerror(errno));
>
> I think we're allowed to use %m here ?
>
changed
> > + printf(_(" --filter=FILENAME read object names from
> file\n"));
>
> Object name filter expression, or something..
>
yes, it is not object names now
> > + * getline is originaly GNU function, and should not be everywhere
> still.
> originally
>
> > + * Use own reduced implementation.
>
> Did you "reduce" this from another implementation? Where?
> What is its license ?
>
> Maybe a line-reader already exists in the frontend (?) .. or maybe it
> should.
>
everywhere else is used a function fgets. Currently pg_getline is used just
on only one place, so I don't think so moving it to some common part is
maybe premature.
Maybe it can be used as replacement of some fgets calls, but then it is
different topic, I think.
Thank you for comments, attached updated patch
Regards
Pavel
> --
> Justin
>
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index 2f0807e912..332f0c312f 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -813,6 +813,15 @@ PostgreSQL documentation
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><option>--filter=<replaceable class="parameter">filename</replaceable></option></term>
+ <listitem>
+ <para>
+ Read filters from file. Format "(+|-)(tnfd) objectname:
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><option>--load-via-partition-root</option></term>
<listitem>
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index dfe43968b8..9e76189635 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -290,6 +290,8 @@ static void appendReloptionsArrayAH(PQExpBuffer buffer, const char *reloptions,
static char *get_synchronized_snapshot(Archive *fout);
static void setupDumpWorker(Archive *AHX);
static TableInfo *getRootTableInfo(TableInfo *tbinfo);
+static size_t pg_getline(char **lineptr, size_t *n, FILE *fp);
+static void invalid_filter_format(char *message, char *filename, char *line, int lineno);
int
@@ -364,6 +366,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},
@@ -603,6 +606,135 @@ main(int argc, char **argv)
optarg);
break;
+ case 12: /* filter implementation */
+ {
+ FILE *f;
+ char *line;
+ ssize_t chars;
+ size_t line_size = 1024;
+ int lineno = 0;
+
+ /* use "-" as symbol for stdin */
+ if (strcmp(optarg, "-") != 0)
+ {
+ f = fopen(optarg, "r");
+ if (!f)
+ fatal("could not open the input file \"%s\": %m",
+ optarg);
+ }
+ else
+ f = stdin;
+
+ line = pg_malloc(line_size);
+
+ while ((chars = pg_getline(&line, &line_size, f)) != -1)
+ {
+ bool is_include;
+ char objecttype;
+ char *objectname;
+
+ lineno += 1;
+
+ if (line[chars - 1] == '\n')
+ line[chars - 1] = '\0';
+
+ /* ignore empty rows */
+ if (*line == '\0')
+ continue;
+
+ if (chars < 2)
+ invalid_filter_format("too short line",
+ optarg,
+ line,
+ lineno);
+
+ if (line[0] == '+')
+ is_include = true;
+ else if (line[0] == '-')
+ is_include = false;
+ else
+ invalid_filter_format("invalid option type (use [+-]",
+ optarg,
+ line,
+ lineno);
+
+ objecttype = line[1];
+ objectname = &line[2];
+
+ /* skip initial spaces */
+ while (isspace(*objectname))
+ objectname++;
+
+ if (*objectname == '\0')
+ invalid_filter_format("missing object name",
+ optarg,
+ line,
+ lineno);
+
+ if (objecttype == 't')
+ {
+ if (is_include)
+ {
+ simple_string_list_append(&table_include_patterns,
+ objectname);
+ dopt.include_everything = false;
+ }
+ else
+ simple_string_list_append(&table_exclude_patterns,
+ objectname);
+ }
+ else if (objecttype == 'n')
+ {
+ if (is_include)
+ {
+ simple_string_list_append(&schema_include_patterns,
+ objectname);
+ dopt.include_everything = false;
+ }
+ else
+ simple_string_list_append(&schema_exclude_patterns,
+ objectname);
+ }
+ else if (objecttype == 'd')
+ {
+ if (is_include)
+ invalid_filter_format("include filter is not supported for this type of object",
+ optarg,
+ line,
+ lineno);
+ else
+ simple_string_list_append(&tabledata_exclude_patterns,
+ objectname);
+ }
+ else if (objecttype == 'f')
+ {
+ if (is_include)
+ simple_string_list_append(&foreign_servers_include_patterns,
+ objectname);
+ else
+ invalid_filter_format("exclude filter is not supported for this type of object",
+ optarg,
+ line,
+ lineno);
+ }
+ else
+ invalid_filter_format("invalid object type (use [tndf])",
+ optarg,
+ line,
+ lineno);
+ }
+
+ if (ferror(f))
+ fatal("could not read from file \"%s\": %m",
+ f == stdin ? "stdin" : optarg);
+
+ if (f != stdin)
+ fclose(f);
+
+ pg_free(line);
+ }
+ break;
+
default:
fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
exit_nicely(1);
@@ -1022,6 +1154,7 @@ 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 read object name filter expressions from 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"
@@ -18647,3 +18780,67 @@ appendReloptionsArrayAH(PQExpBuffer buffer, const char *reloptions,
if (!res)
pg_log_warning("could not parse reloptions array");
}
+
+/*
+ * getline is originaly GNU function, and should not be everywhere still.
+ * Use own reduced implementation.
+ */
+static size_t
+pg_getline(char **lineptr, size_t *n, FILE *fp)
+{
+ size_t total_chars = 0;
+
+ while (!feof(fp) && !ferror(fp))
+ {
+ char *str;
+ size_t chars;
+
+ str = fgets(*lineptr + total_chars,
+ *n - total_chars,
+ fp);
+
+ if (ferror(fp))
+ return -1;
+
+ if (str)
+ {
+ chars = strlen(str);
+ total_chars += chars;
+
+ if (chars > 0 && str[chars - 1] == '\n')
+ return total_chars;
+
+ /* check, if there is good enough space for next content */
+ if (*n - total_chars < 2)
+ {
+ *n += 1024;
+ *lineptr = pg_realloc(*lineptr, *n);
+ }
+ }
+ else
+ break;
+ }
+
+ if (ferror(fp))
+ return -1;
+
+ return total_chars > 0 ? total_chars : -1;
+}
+
+/*
+ * Print error message and exit.
+ */
+static void
+invalid_filter_format(char *message, char *filename, char *line, int lineno)
+{
+ char *displayname;
+
+ displayname = *filename == '-' ? "stdin" : filename;
+
+ pg_log_error("invalid format of filter file \"%s\": %s",
+ displayname,
+ message);
+
+ fprintf(stderr, "%d: %s\n", lineno, line);
+ exit_nicely(1);
+}
