Hi

I am sending updated version

news:

* strict-names everywhere
* checking table names in pg_dump simplified - not necessary to create
single query
* pg_restore support

Regards

Pavel


2015-08-13 9:17 GMT+02:00 Pavel Stehule <pavel.steh...@gmail.com>:

> Hi
>
> 2015-07-30 12:44 GMT+02:00 Heikki Linnakangas <hlinn...@iki.fi>:
>
>> On 07/25/2015 07:08 PM, Pavel Stehule wrote:
>>
>>> I am sending a new patch - without checking wildcard chars.
>>>
>>
>> The documentation says the option is called --strict-names, while the
>> code has --strict-mode. I like --strict-names more, "mode" seems redundant,
>> and it's not clear what it's strict about.
>>
>
> ok
>
>>
>> For symmetry, it would be good to also support this option in pg_restore.
>> It seems even more useful there.
>>
>
> I'll do it
>
>>
>> Can we do better than issuing a separate query for each table/schema
>> name? The performance of this isn't very important, but still it seems like
>> you could fairly easily refactor the code to avoid that. Perhaps return an
>> extra constant for part of the UNION to distinguish which result row came
>> from which pattern, and check that at least one row is returned for each.
>>
>
> I did few tests and for 1K tables the union is faster about 50ms, but the
> code is much more complex, for 10K tables, the union is significantly
> slower (probably due planning) 2sec x 7sec. So if we are expecting backup
> on not too slow network, then simple solution is winner - Postgres process
> simple read queries quickly.
>
> Regards
>
> Pavel
>
>
>>
>> - Heikki
>>
>>
>
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
new file mode 100644
index 7467e86..7c071fb
*** a/doc/src/sgml/ref/pg_dump.sgml
--- b/doc/src/sgml/ref/pg_dump.sgml
*************** PostgreSQL documentation
*** 545,550 ****
--- 545,566 ----
       </varlistentry>
  
       <varlistentry>
+       <term><option>--strict-names</></term>
+       <listitem>
+        <para>
+         Require that table and/or schema match at least one entity each.
+         Without any entity in the database to be dumped, an error message
+         is printed and dump is aborted.
+        </para>
+        <para>
+         This option has no effect on the exclude table and schema patterns
+         (and also <option>--exclude-table-data</>): not matching any entities
+         isn't considered an error.
+        </para>
+       </listitem>
+      </varlistentry>
+ 
+      <varlistentry>
        <term><option>-T <replaceable class="parameter">table</replaceable></option></term>
        <term><option>--exclude-table=<replaceable class="parameter">table</replaceable></option></term>
        <listitem>
diff --git a/doc/src/sgml/ref/pg_restore.sgml b/doc/src/sgml/ref/pg_restore.sgml
new file mode 100644
index 97e3420..9df7f69
*** a/doc/src/sgml/ref/pg_restore.sgml
--- b/doc/src/sgml/ref/pg_restore.sgml
***************
*** 432,437 ****
--- 432,448 ----
       </varlistentry>
  
       <varlistentry>
+       <term><option>--strict-names</></term>
+       <listitem>
+        <para>
+         Require that table and/or schema and/or function and/or trigger match
+         at least one entity each. Without any entity in the backup file, 
+         an error message is printed and restore is aborted.
+        </para>
+       </listitem>
+      </varlistentry>
+ 
+      <varlistentry>
        <term><option>-T <replaceable class="parameter">trigger</replaceable></option></term>
        <term><option>--trigger=<replaceable class="parameter">trigger</replaceable></option></term>
        <listitem>
diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
new file mode 100644
index d7506e1..52b2b98
*** a/src/bin/pg_dump/dumputils.c
--- b/src/bin/pg_dump/dumputils.c
*************** simple_string_list_append(SimpleStringLi
*** 1220,1225 ****
--- 1220,1226 ----
  		pg_malloc(offsetof(SimpleStringListCell, val) +strlen(val) + 1);
  
  	cell->next = NULL;
+ 	cell->touched = false;
  	strcpy(cell->val, val);
  
  	if (list->tail)
*************** simple_string_list_member(SimpleStringLi
*** 1237,1243 ****
--- 1238,1260 ----
  	for (cell = list->head; cell; cell = cell->next)
  	{
  		if (strcmp(cell->val, val) == 0)
+ 		{
+ 			cell->touched = true;
  			return true;
+ 		}
  	}
  	return false;
  }
+ 
+ const char *
+ simple_string_list_not_touched(SimpleStringList *list)
+ {
+ 	SimpleStringListCell *cell;
+ 
+ 	for (cell = list->head; cell; cell = cell->next)
+ 	{
+ 		if (!cell->touched)
+ 			return cell->val;
+ 	}
+ 	return NULL;
+ }
diff --git a/src/bin/pg_dump/dumputils.h b/src/bin/pg_dump/dumputils.h
new file mode 100644
index b176746..9f31bbc
*** a/src/bin/pg_dump/dumputils.h
--- b/src/bin/pg_dump/dumputils.h
*************** typedef struct SimpleOidList
*** 38,43 ****
--- 38,45 ----
  typedef struct SimpleStringListCell
  {
  	struct SimpleStringListCell *next;
+ 	bool		touched;				/* true, when this string was searched
+ 								      and touched */
  	char		val[FLEXIBLE_ARRAY_MEMBER];		/* null-terminated string here */
  } SimpleStringListCell;
  
*************** extern void set_dump_section(const char
*** 103,107 ****
--- 105,111 ----
  
  extern void simple_string_list_append(SimpleStringList *list, const char *val);
  extern bool simple_string_list_member(SimpleStringList *list, const char *val);
+ extern const char *simple_string_list_not_touched(SimpleStringList *list);
+ 
  
  #endif   /* DUMPUTILS_H */
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
new file mode 100644
index 80df8fc..7126749
*** a/src/bin/pg_dump/pg_backup.h
--- b/src/bin/pg_dump/pg_backup.h
*************** typedef struct _restoreOptions
*** 104,109 ****
--- 104,110 ----
  	int			column_inserts;
  	int			if_exists;
  	int			no_security_labels;		/* Skip security label entries */
+ 	int			strict_names;
  
  	const char *filename;
  	int			dataOnly;
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
new file mode 100644
index 8f1f6c1..2344937
*** a/src/bin/pg_dump/pg_backup_archiver.c
--- b/src/bin/pg_dump/pg_backup_archiver.c
*************** static void reduce_dependencies(ArchiveH
*** 108,113 ****
--- 108,116 ----
  static void mark_create_done(ArchiveHandle *AH, TocEntry *te);
  static void inhibit_data_for_failed_table(ArchiveHandle *AH, TocEntry *te);
  
+ static void StrictNamesCheck(RestoreOptions *ropt);
+ 
+ 
  /*
   * Allocate a new DumpOptions block containing all default values.
   */
*************** SetArchiveRestoreOptions(Archive *AHX, R
*** 284,289 ****
--- 287,296 ----
  
  		te->reqs = _tocEntryRequired(te, curSection, ropt);
  	}
+ 
+ 	/* Enforce strict names checking */
+ 	if (ropt->strict_names)
+ 		StrictNamesCheck(ropt);
  }
  
  /* Public */
*************** PrintTOCSummary(Archive *AHX, RestoreOpt
*** 1104,1109 ****
--- 1111,1120 ----
  		}
  	}
  
+ 	/* Enforce strict names checking */
+ 	if (ropt->strict_names)
+ 		StrictNamesCheck(ropt);
+ 
  	if (ropt->filename)
  		RestoreOutput(AH, sav);
  }
*************** processStdStringsEntry(ArchiveHandle *AH
*** 2612,2617 ****
--- 2623,2671 ----
  					  te->defn);
  }
  
+ static void
+ StrictNamesCheck(RestoreOptions *ropt)
+ {
+ 	const char *missing_name;
+ 
+ 	Assert(ropt->strict_names);
+ 
+ 	if (ropt->schemaNames.head != NULL)
+ 	{
+ 		missing_name = simple_string_list_not_touched(&ropt->schemaNames);
+ 		if (missing_name != NULL)
+ 			exit_horribly(modulename, "Schema \"%s\" not found.\n", missing_name);
+ 	}
+ 
+ 	if (ropt->tableNames.head != NULL)
+ 	{
+ 		missing_name = simple_string_list_not_touched(&ropt->tableNames);
+ 		if (missing_name != NULL)
+ 			exit_horribly(modulename, "Table \"%s\" not found.\n", missing_name);
+ 	}
+ 
+ 	if (ropt->indexNames.head != NULL)
+ 	{
+ 		missing_name = simple_string_list_not_touched(&ropt->indexNames);
+ 		if (missing_name != NULL)
+ 			exit_horribly(modulename, "Index \"%s\" not found.\n", missing_name);
+ 	}
+ 
+ 	if (ropt->functionNames.head != NULL)
+ 	{
+ 		missing_name = simple_string_list_not_touched(&ropt->functionNames);
+ 		if (missing_name != NULL)
+ 			exit_horribly(modulename, "Function \"%s\" not found.\n", missing_name);
+ 	}
+ 
+ 	if (ropt->triggerNames.head != NULL)
+ 	{
+ 		missing_name = simple_string_list_not_touched(&ropt->triggerNames);
+ 		if (missing_name != NULL)
+ 			exit_horribly(modulename, "Trigger \"%s\" not found.\n", missing_name);
+ 	}
+ }
+ 
  static teReqs
  _tocEntryRequired(TocEntry *te, teSection curSection, RestoreOptions *ropt)
  {
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
new file mode 100644
index 87dadbf..3914cfb
*** a/src/bin/pg_dump/pg_dump.c
--- b/src/bin/pg_dump/pg_dump.c
*************** static const char *username_subquery;
*** 97,102 ****
--- 97,105 ----
  /* obsolete as of 7.3: */
  static Oid	g_last_builtin_oid; /* value of the last builtin oid */
  
+ /* The specified names/patterns should to match at least one entity */
+ static int	strict_names = 0;
+ 
  /*
   * Object inclusion/exclusion lists
   *
*************** static void setup_connection(Archive *AH
*** 131,140 ****
  static ArchiveFormat parseArchiveFormat(const char *format, ArchiveMode *mode);
  static void expand_schema_name_patterns(Archive *fout,
  							SimpleStringList *patterns,
! 							SimpleOidList *oids);
  static void expand_table_name_patterns(Archive *fout,
  						   SimpleStringList *patterns,
! 						   SimpleOidList *oids);
  static NamespaceInfo *findNamespace(Archive *fout, Oid nsoid, Oid objoid);
  static void dumpTableData(Archive *fout, DumpOptions *dopt, TableDataInfo *tdinfo);
  static void refreshMatViewData(Archive *fout, TableDataInfo *tdinfo);
--- 134,145 ----
  static ArchiveFormat parseArchiveFormat(const char *format, ArchiveMode *mode);
  static void expand_schema_name_patterns(Archive *fout,
  							SimpleStringList *patterns,
! 							SimpleOidList *oids,
! 							bool strict_names);
  static void expand_table_name_patterns(Archive *fout,
  						   SimpleStringList *patterns,
! 						   SimpleOidList *oids,
! 						   bool strict_names);
  static NamespaceInfo *findNamespace(Archive *fout, Oid nsoid, Oid objoid);
  static void dumpTableData(Archive *fout, DumpOptions *dopt, TableDataInfo *tdinfo);
  static void refreshMatViewData(Archive *fout, TableDataInfo *tdinfo);
*************** main(int argc, char **argv)
*** 333,338 ****
--- 338,344 ----
  		{"section", required_argument, NULL, 5},
  		{"serializable-deferrable", no_argument, &dopt.serializable_deferrable, 1},
  		{"snapshot", required_argument, NULL, 6},
+ 		{"strict-mode", no_argument, &strict_names, 1},
  		{"use-set-session-authorization", no_argument, &dopt.use_setsessauth, 1},
  		{"no-security-labels", no_argument, &dopt.no_security_labels, 1},
  		{"no-synchronized-snapshots", no_argument, &dopt.no_synchronized_snapshots, 1},
*************** main(int argc, char **argv)
*** 690,716 ****
  	if (schema_include_patterns.head != NULL)
  	{
  		expand_schema_name_patterns(fout, &schema_include_patterns,
! 									&schema_include_oids);
  		if (schema_include_oids.head == NULL)
  			exit_horribly(NULL, "No matching schemas were found\n");
  	}
  	expand_schema_name_patterns(fout, &schema_exclude_patterns,
! 								&schema_exclude_oids);
  	/* non-matching exclusion patterns aren't an error */
  
  	/* Expand table selection patterns into OID lists */
  	if (table_include_patterns.head != NULL)
  	{
  		expand_table_name_patterns(fout, &table_include_patterns,
! 								   &table_include_oids);
  		if (table_include_oids.head == NULL)
  			exit_horribly(NULL, "No matching tables were found\n");
  	}
  	expand_table_name_patterns(fout, &table_exclude_patterns,
! 							   &table_exclude_oids);
  
  	expand_table_name_patterns(fout, &tabledata_exclude_patterns,
! 							   &tabledata_exclude_oids);
  
  	/* non-matching exclusion patterns aren't an error */
  
--- 696,727 ----
  	if (schema_include_patterns.head != NULL)
  	{
  		expand_schema_name_patterns(fout, &schema_include_patterns,
! 									&schema_include_oids,
! 									strict_names);
  		if (schema_include_oids.head == NULL)
  			exit_horribly(NULL, "No matching schemas were found\n");
  	}
  	expand_schema_name_patterns(fout, &schema_exclude_patterns,
! 								&schema_exclude_oids,
! 								false);
  	/* non-matching exclusion patterns aren't an error */
  
  	/* Expand table selection patterns into OID lists */
  	if (table_include_patterns.head != NULL)
  	{
  		expand_table_name_patterns(fout, &table_include_patterns,
! 								   &table_include_oids,
! 								   strict_names);
  		if (table_include_oids.head == NULL)
  			exit_horribly(NULL, "No matching tables were found\n");
  	}
  	expand_table_name_patterns(fout, &table_exclude_patterns,
! 							   &table_exclude_oids,
! 							   false);
  
  	expand_table_name_patterns(fout, &tabledata_exclude_patterns,
! 							   &tabledata_exclude_oids,
! 							   false);
  
  	/* non-matching exclusion patterns aren't an error */
  
*************** help(const char *progname)
*** 905,910 ****
--- 916,923 ----
  	printf(_("  --section=SECTION            dump named section (pre-data, data, or post-data)\n"));
  	printf(_("  --serializable-deferrable    wait until the dump can run without anomalies\n"));
  	printf(_("  --snapshot=SNAPSHOT          use given synchronous snapshot for the dump\n"));
+ 	printf(_("  --strict-names               require table and/or schema include patterns to\n"
+ 			 "                               match at least one entity each\n"));
  	printf(_("  --use-set-session-authorization\n"
  			 "                               use SET SESSION AUTHORIZATION commands instead of\n"
  			 "                               ALTER OWNER commands to set ownership\n"));
*************** parseArchiveFormat(const char *format, A
*** 1135,1141 ****
  static void
  expand_schema_name_patterns(Archive *fout,
  							SimpleStringList *patterns,
! 							SimpleOidList *oids)
  {
  	PQExpBuffer query;
  	PGresult   *res;
--- 1148,1155 ----
  static void
  expand_schema_name_patterns(Archive *fout,
  							SimpleStringList *patterns,
! 							SimpleOidList *oids,
! 							bool strict_names)
  {
  	PQExpBuffer query;
  	PGresult   *res;
*************** expand_schema_name_patterns(Archive *fou
*** 1151,1188 ****
  	query = createPQExpBuffer();
  
  	/*
! 	 * We use UNION ALL rather than UNION; this might sometimes result in
! 	 * duplicate entries in the OID list, but we don't care.
  	 */
  
  	for (cell = patterns->head; cell; cell = cell->next)
  	{
- 		if (cell != patterns->head)
- 			appendPQExpBufferStr(query, "UNION ALL\n");
  		appendPQExpBuffer(query,
  						  "SELECT oid FROM pg_catalog.pg_namespace n\n");
  		processSQLNamePattern(GetConnection(fout), query, cell->val, false,
  							  false, NULL, "n.nspname", NULL, NULL);
- 	}
  
! 	res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
  
! 	for (i = 0; i < PQntuples(res); i++)
! 	{
! 		simple_oid_list_append(oids, atooid(PQgetvalue(res, i, 0)));
  	}
  
- 	PQclear(res);
  	destroyPQExpBuffer(query);
  }
  
  /*
   * Find the OIDs of all tables matching the given list of patterns,
!  * and append them to the given OID list.
   */
  static void
  expand_table_name_patterns(Archive *fout,
! 						   SimpleStringList *patterns, SimpleOidList *oids)
  {
  	PQExpBuffer query;
  	PGresult   *res;
--- 1165,1205 ----
  	query = createPQExpBuffer();
  
  	/*
! 	 * This might sometimes result in duplicate entries in the OID list,
! 	 * but we don't care.
  	 */
  
  	for (cell = patterns->head; cell; cell = cell->next)
  	{
  		appendPQExpBuffer(query,
  						  "SELECT oid FROM pg_catalog.pg_namespace n\n");
  		processSQLNamePattern(GetConnection(fout), query, cell->val, false,
  							  false, NULL, "n.nspname", NULL, NULL);
  
! 		res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
! 		if (strict_names && PQntuples(res) == 0)
! 			exit_horribly(NULL, "Schema \"%s\" not found.\n", cell->val);
  
! 		for (i = 0; i < PQntuples(res); i++)
! 		{
! 			simple_oid_list_append(oids, atooid(PQgetvalue(res, i, 0)));
! 		}
! 
! 		PQclear(res);
! 		resetPQExpBuffer(query);
  	}
  
  	destroyPQExpBuffer(query);
  }
  
  /*
   * Find the OIDs of all tables matching the given list of patterns,
!  * and append them to the given OID list. 
   */
  static void
  expand_table_name_patterns(Archive *fout,
! 						   SimpleStringList *patterns, SimpleOidList *oids,
! 						   bool strict_names)
  {
  	PQExpBuffer query;
  	PGresult   *res;
*************** expand_table_name_patterns(Archive *fout
*** 1195,1208 ****
  	query = createPQExpBuffer();
  
  	/*
! 	 * We use UNION ALL rather than UNION; this might sometimes result in
! 	 * duplicate entries in the OID list, but we don't care.
  	 */
  
  	for (cell = patterns->head; cell; cell = cell->next)
  	{
- 		if (cell != patterns->head)
- 			appendPQExpBufferStr(query, "UNION ALL\n");
  		appendPQExpBuffer(query,
  						  "SELECT c.oid"
  						  "\nFROM pg_catalog.pg_class c"
--- 1212,1223 ----
  	query = createPQExpBuffer();
  
  	/*
! 	 * this might sometimes result in duplicate entries in the OID list,
! 	 * but we don't care.
  	 */
  
  	for (cell = patterns->head; cell; cell = cell->next)
  	{
  		appendPQExpBuffer(query,
  						  "SELECT c.oid"
  						  "\nFROM pg_catalog.pg_class c"
*************** expand_table_name_patterns(Archive *fout
*** 1213,1228 ****
  		processSQLNamePattern(GetConnection(fout), query, cell->val, true,
  							  false, "n.nspname", "c.relname", NULL,
  							  "pg_catalog.pg_table_is_visible(c.oid)");
- 	}
  
! 	res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
  
! 	for (i = 0; i < PQntuples(res); i++)
! 	{
! 		simple_oid_list_append(oids, atooid(PQgetvalue(res, i, 0)));
  	}
  
- 	PQclear(res);
  	destroyPQExpBuffer(query);
  }
  
--- 1228,1247 ----
  		processSQLNamePattern(GetConnection(fout), query, cell->val, true,
  							  false, "n.nspname", "c.relname", NULL,
  							  "pg_catalog.pg_table_is_visible(c.oid)");
  
! 		res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
! 		if (strict_names && PQntuples(res) == 0)
! 			exit_horribly(NULL, "Table \"%s\" not found.\n", cell->val);
  
! 		for (i = 0; i < PQntuples(res); i++)
! 		{
! 			simple_oid_list_append(oids, atooid(PQgetvalue(res, i, 0)));
! 		}
! 
! 		PQclear(res);
! 		resetPQExpBuffer(query);
  	}
  
  	destroyPQExpBuffer(query);
  }
  
diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
new file mode 100644
index b129488..75c08b9
*** a/src/bin/pg_dump/pg_restore.c
--- b/src/bin/pg_dump/pg_restore.c
*************** main(int argc, char **argv)
*** 77,82 ****
--- 77,83 ----
  	static int	outputNoTablespaces = 0;
  	static int	use_setsessauth = 0;
  	static int	no_security_labels = 0;
+ 	static int	strict_names = 0;
  
  	struct option cmdopts[] = {
  		{"clean", 0, NULL, 'c'},
*************** main(int argc, char **argv)
*** 118,123 ****
--- 119,125 ----
  		{"no-tablespaces", no_argument, &outputNoTablespaces, 1},
  		{"role", required_argument, NULL, 2},
  		{"section", required_argument, NULL, 3},
+ 		{"strict-names", no_argument, &strict_names, 1},
  		{"use-set-session-authorization", no_argument, &use_setsessauth, 1},
  		{"no-security-labels", no_argument, &no_security_labels, 1},
  
*************** main(int argc, char **argv)
*** 345,350 ****
--- 347,353 ----
  		exit_nicely(1);
  	}
  	opts->if_exists = if_exists;
+ 	opts->strict_names = strict_names;
  
  	if (opts->formatName)
  	{
*************** usage(const char *progname)
*** 467,472 ****
--- 470,477 ----
  	printf(_("  --no-security-labels         do not restore security labels\n"));
  	printf(_("  --no-tablespaces             do not restore tablespace assignments\n"));
  	printf(_("  --section=SECTION            restore named section (pre-data, data, or post-data)\n"));
+ 	printf(_("  --strict-names               require table and/or schema include patterns to\n"
+ 			 "                               match at least one entity each\n"));
  	printf(_("  --use-set-session-authorization\n"
  			 "                               use SET SESSION AUTHORIZATION commands instead of\n"
  			 "                               ALTER OWNER commands to set ownership\n"));
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to