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