Pavel Stehule <pavel.steh...@gmail.com> writes:
>
> 2015-03-23 17:11 GMT+01:00 Pavel Stehule <pavel.steh...@gmail.com>:
>
>> Hi
>>
>> 2015-03-15 16:09 GMT+01:00 Tom Lane <t...@sss.pgh.pa.us>:
>>
>>> Pavel Stehule <pavel.steh...@gmail.com> writes:
>>> > other variant, I hope better than previous. We can introduce new long
>>> > option "--strict". With this active option, every pattern specified by
>>> -t
>>> > option have to have identifies exactly only one table. It can be used
>>> for
>>> > any other "should to exists" patterns - schemas. Initial implementation
>>> in
>>> > attachment.
>>>
>>> I think this design is seriously broken. If I have '-t foo*' the code
>>> should not prevent that from matching multiple tables. What would the use
>>> case for such a restriction be?
>>>
>>> What would make sense to me is one or both of these ideas:
>>>
>>> * require a match for a wildcard-free -t switch
>>>
>>> * require at least one (not "exactly one") match for a wildcarded -t
>>> switch.
>>>
>>
>>
>> attached initial implementation
>>
>
> updated version - same mechanism should be used for schema
Hello,
I think this is a bit over-engineered (apart from the fact that
processSQLNamePattern is also used in two dozen of places in
psql/describe.c and all of them must be touched for this patch to
compile).
Also, the new --table-if-exists options seems to be doing what the old
--table did, and I'm not really sure I underestand what --table does
now.
I propose instead to add a separate new option --strict-include, without
argument, that only controls the behavior when an include pattern didn't
find any table (or schema).
Please see attached patch.
--
Alex
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
new file mode 100644
index a6e7b08..da33331
*** a/doc/src/sgml/ref/pg_dump.sgml
--- b/doc/src/sgml/ref/pg_dump.sgml
*************** PostgreSQL documentation
*** 365,370 ****
--- 365,377 ----
<xref linkend="pg-dump-examples" endterm="pg-dump-examples-title">.
</para>
+ <para>
+ When <replaceable class="parameter">schema</replaceable> doesn't
+ specify a pattern, it must match an exact schema name in the database
+ to be dumped. In case of a pattern, the behavior is controlled
+ by <option>--strict-include</> option.
+ </para>
+
<note>
<para>
When <option>-n</> is specified, <application>pg_dump</application>
*************** PostgreSQL documentation
*** 515,520 ****
--- 522,534 ----
</para>
<para>
+ When <replaceable class="parameter">table</replaceable> doesn't
+ specify a pattern, it must match an exact table name in the database
+ to be dumped. In case of a pattern, the behavior is controlled
+ by <option>--strict-include</> option.
+ </para>
+
+ <para>
The <option>-n</> and <option>-N</> switches have no effect when
<option>-t</> is used, because tables selected by <option>-t</> will
be dumped regardless of those switches, and non-table objects will not
*************** PostgreSQL documentation
*** 902,907 ****
--- 916,938 ----
</para>
</listitem>
</varlistentry>
+
+ <varlistentry>
+ <term><option>--strict-include</></term>
+ <listitem>
+ <para>
+ Require that table and/or schema include patterns match at least one
+ entity each. If any of the include patterns specified doesn't match
+ 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>--use-set-session-authorization</></term>
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
new file mode 100644
index 80df8fc..f384c20
*** a/src/bin/pg_dump/pg_backup.h
--- b/src/bin/pg_dump/pg_backup.h
*************** typedef struct _dumpOptions
*** 178,183 ****
--- 178,184 ----
int outputNoTablespaces;
int use_setsessauth;
int enable_row_security;
+ int strict_include;
/* default, if no "inclusion" switches appear, is to dump everything */
bool include_everything;
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
new file mode 100644
index dccb472..4f62085
*** a/src/bin/pg_dump/pg_dump.c
--- b/src/bin/pg_dump/pg_dump.c
*************** 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);
--- 131,142 ----
static ArchiveFormat parseArchiveFormat(const char *format, ArchiveMode *mode);
static void expand_schema_name_patterns(Archive *fout,
SimpleStringList *patterns,
! SimpleOidList *oids,
! bool strict);
static void expand_table_name_patterns(Archive *fout,
SimpleStringList *patterns,
! SimpleOidList *oids,
! bool strict);
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 ****
--- 335,341 ----
{"section", required_argument, NULL, 5},
{"serializable-deferrable", no_argument, &dopt.serializable_deferrable, 1},
{"snapshot", required_argument, NULL, 6},
+ {"strict-include", no_argument, &dopt.strict_include, 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)
*** 689,715 ****
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 */
--- 692,714 ----
if (schema_include_patterns.head != NULL)
{
expand_schema_name_patterns(fout, &schema_include_patterns,
! &schema_include_oids, dopt.strict_include);
}
expand_schema_name_patterns(fout, &schema_exclude_patterns,
! &schema_exclude_oids, true);
/* 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, dopt.strict_include);
}
expand_table_name_patterns(fout, &table_exclude_patterns,
! &table_exclude_oids, true);
expand_table_name_patterns(fout, &tabledata_exclude_patterns,
! &tabledata_exclude_oids, true);
/* non-matching exclusion patterns aren't an error */
*************** help(const char *progname)
*** 904,909 ****
--- 903,910 ----
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-include 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
*** 1133,1139 ****
static void
expand_schema_name_patterns(Archive *fout,
SimpleStringList *patterns,
! SimpleOidList *oids)
{
PQExpBuffer query;
PGresult *res;
--- 1134,1141 ----
static void
expand_schema_name_patterns(Archive *fout,
SimpleStringList *patterns,
! SimpleOidList *oids,
! bool strict)
{
PQExpBuffer query;
PGresult *res;
*************** expand_schema_name_patterns(Archive *fou
*** 1155,1176 ****
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);
}
--- 1157,1183 ----
for (cell = patterns->head; cell; cell = cell->next)
{
! resetPQExpBuffer(query);
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 (PQntuples(res) < 1 &&
! (strict || strcspn(cell->val, "?*") == strlen(cell->val)))
! {
! exit_horribly(NULL, "Schema(s) not found: %s\n", cell->val);
! }
! for (i = 0; i < PQntuples(res); i++)
! {
! simple_oid_list_append(oids, atooid(PQgetvalue(res, i, 0)));
! }
! PQclear(res);
! }
destroyPQExpBuffer(query);
}
*************** expand_schema_name_patterns(Archive *fou
*** 1180,1186 ****
*/
static void
expand_table_name_patterns(Archive *fout,
! SimpleStringList *patterns, SimpleOidList *oids)
{
PQExpBuffer query;
PGresult *res;
--- 1187,1194 ----
*/
static void
expand_table_name_patterns(Archive *fout,
! SimpleStringList *patterns, SimpleOidList *oids,
! bool strict)
{
PQExpBuffer query;
PGresult *res;
*************** expand_table_name_patterns(Archive *fout
*** 1199,1206 ****
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"
--- 1207,1213 ----
for (cell = patterns->head; cell; cell = cell->next)
{
! resetPQExpBuffer(query);
appendPQExpBuffer(query,
"SELECT c.oid"
"\nFROM pg_catalog.pg_class c"
*************** expand_table_name_patterns(Archive *fout
*** 1208,1226 ****
"\nWHERE c.relkind in ('%c', '%c', '%c', '%c', '%c')\n",
RELKIND_RELATION, RELKIND_SEQUENCE, RELKIND_VIEW,
RELKIND_MATVIEW, RELKIND_FOREIGN_TABLE);
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);
}
--- 1215,1240 ----
"\nWHERE c.relkind in ('%c', '%c', '%c', '%c', '%c')\n",
RELKIND_RELATION, RELKIND_SEQUENCE, RELKIND_VIEW,
RELKIND_MATVIEW, RELKIND_FOREIGN_TABLE);
+
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 (PQntuples(res) < 1 &&
! (strict || strcspn(cell->val, "?*") == strlen(cell->val)))
! {
! exit_horribly(NULL, "Table(s) not found: %s\n", cell->val);
! }
! for (i = 0; i < PQntuples(res); i++)
! {
! simple_oid_list_append(oids, atooid(PQgetvalue(res, i, 0)));
! }
!
! PQclear(res);
}
destroyPQExpBuffer(query);
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
