so 27. 6. 2020 v 14:55 odesílatel vignesh C <vignes...@gmail.com> napsal:

> On Thu, Jun 11, 2020 at 1:07 PM Pavel Stehule <pavel.steh...@gmail.com>
> wrote:
> >
> > Thank you for comments, attached updated patch
> >
>
> Few comments:
> +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);
> +}
>
I think fclose is missing here.
>

done


>
> +                                               if (line[chars - 1] ==
> '\n')
> +                                                       line[chars - 1] =
> '\0';
> Should we check for '\r' also to avoid failures in some platforms.
>

I checked other usage of fgets in Postgres source code, and everywhere is
used test on \n

When I did some fast research, then
https://stackoverflow.com/questions/12769289/carriage-return-by-fgets \r in
this case should be thrown by libc on Microsoft

https://stackoverflow.com/questions/2061334/fgets-linux-vs-mac

\n should be on Mac OS X .. 2001 year .. I am not sure if Mac OS 9 should
be supported.




>
> +     <varlistentry>
> +      <term><option>--filter=<replaceable
> class="parameter">filename</replaceable></option></term>
> +      <listitem>
> +       <para>
> +        Read filters from file. Format "(+|-)(tnfd) objectname:
> +       </para>
> +      </listitem>
> +     </varlistentry>
>
> I felt some documentation is missing here. We could include,
> options tnfd is for controlling table, schema, foreign server data &
> table exclude patterns.
>

I have a plan to completate doc when the design is completed. It was not
clear if people prefer long or short forms of option names.


> Instead of using tnfd, if we could use the same options as existing
> pg_dump options it will be less confusing.
>

it almost same

+-t .. tables
+-n schema
-d exclude data .. there is not short option for --exclude-table-data
+f include foreign table .. there is not short option for
--include-foreign-data

So still, there is a opened question if use +-tnfd system, or system based
on long option

table foo
exclude-table foo
schema xx
exclude-schema xx
include-foreign-data yyy
exclude-table-data zzz


Typically these files will be generated by scripts and processed via pipe,
so there I see just two arguments for and aginst:

short format - there is less probability to do typo error (but there is not
full consistency with pg_dump options)
long format - it is self documented (and there is full consistency with
pg_dump)

In this case I prefer short form .. it is more comfortable for users, and
there are only a few variants, so it is not necessary to use too verbose
language (design). But my opinion is not aggressively strong and I'll
accept any common agreement.

Regards

Updated patch attached



> Regards,
> Vignesh
> EnterpriseDB: http://www.enterprisedb.com
>
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 a41a3db876..a3f8bebf52 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 exit_invalid_filter_format(FILE *fp, char *filename, char *message, 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,141 @@ 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)
+							exit_invalid_filter_format(f,
+													   optarg,
+													   "too short line",
+													   line,
+													   lineno);
+
+						if (line[0] == '+')
+							is_include = true;
+						else if (line[0] == '-')
+							is_include = false;
+						else
+							exit_invalid_filter_format(f,
+													   optarg,
+													   "invalid option type (use [+-]",
+													   line,
+													   lineno);
+
+						objecttype = line[1];
+						objectname = &line[2];
+
+						/* skip initial spaces */
+						while (isspace(*objectname))
+							objectname++;
+
+						if (*objectname == '\0')
+							exit_invalid_filter_format(f,
+													   optarg,
+													   "missing object name",
+													   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)
+								exit_invalid_filter_format(f,
+														   optarg,
+														   "include filter is not supported for this type of object",
+														   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
+								exit_invalid_filter_format(f,
+														   optarg,
+														   "exclude filter is not supported for this type of object",
+														   line,
+														   lineno);
+						}
+						else
+							exit_invalid_filter_format(f,
+													   optarg,
+													   "invalid object type (use [tndf])",
+													   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 +1160,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"
@@ -18624,3 +18763,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
+exit_invalid_filter_format(FILE *fp, char *filename, char *message, char *line, int lineno)
+{
+	pg_log_error("invalid format of filter file \"%s\": %s",
+				 *filename == '-' ? "stdin" : filename,
+				 message);
+
+	fprintf(stderr, "%d: %s\n", lineno, line);
+
+	if (fp != stdin)
+		fclose(fp);
+
+	exit_nicely(-1);
+}

Reply via email to