Hi
I am sending updated version. It implements new long option
"--strict-names". If this option is used, then for any entered name
(without any wildcard char) must be found least one object. This option has
not impact on patters (has wildcards chars). When this option is not used,
then behave is 100% same as before (with same numbers of SQL queries for -t
option). It is based on Oleksandr's documentation (and lightly modified
philosophy), and cleaned my previous patch. A test on wildchard existency
"strcspn(cell->val, "?*")" cannot be used, because it doesn't calculate
quotes (but a replace has few lines only).
There is a possibility to remove a wildcard char test and require least one
entry for patters too. But I am thinking, when somebody explicitly uses any
wildcard, then he calculate with a possibility of empty result.
Regards
Pavel
2015-07-09 22:48 GMT+02:00 Pavel Stehule <pavel.steh...@gmail.com>:
>
>
> 2015-07-08 5:36 GMT+02:00 Fujii Masao <masao.fu...@gmail.com>:
>
>> On Sat, May 23, 2015 at 1:41 AM, Pavel Stehule <pavel.steh...@gmail.com>
>> wrote:
>> >
>> >
>> > 2015-05-22 18:34 GMT+02:00 Tom Lane <t...@sss.pgh.pa.us>:
>> >>
>> >> Oleksandr Shulgin <oleksandr.shul...@zalando.de> writes:
>> >> > 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'm pretty sure we had agreed *not* to change the default behavior of
>> -t.
>> >>
>> >> > 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).
>> >>
>> >> If we do it as a separate option, then it necessarily changes the
>> behavior
>> >> for *each* -t switch in the call. Can anyone show a common use-case
>> where
>> >> that's no good, and you need separate behavior for each of several -t
>> >> switches? If not, I like the simplicity of this approach. (Perhaps
>> the
>> >> switch name could use some bikeshedding, though.)
>> >
>> >
>> > it is near to one proposal
>> >
>> > implement only new long option "--required-table"
>>
>> There is no updated version of the patch. So I marked this patch
>> as "Waiting on Author".
>>
>
> tomorrow I'll return to this topic.
>
>
>>
>> One very simple question is, doesn't -n option have very similar problem?
>> Also what about -N, -T and --exclude-table-data? Not sure if we need to
>> handle them in the similar way as you proposed.
>>
>
> hard to say - I understand to your motivation, and it can signalize some
> inconsistency in environment, but it has not same negative impact as same
> problem with -t -n options.
>
> This is maybe place for warning message with possibility to disable this
> warning. But maybe it is premature optimization?
>
> Next way is introduction of "strictness" option - default can be
> equivalent of current, safe can check only tables required for dump, strict
> can check all.
>
>
>>
>> Isn't it sufficient to only emit the warning message to stderr if there
>> is no table matching the pattern specified in -t?
>>
>
> I prefer raising error in this case.
>
> 1. I am thinking so missing tables for dump signalize important
> inconsistency in environment and it is important bug
> 2. The warning is not simply detected (and processed) in bash scripts.
>
> Regards
>
> Pavel
>
>
>>
>> Regards,
>>
>> --
>> Fujii Masao
>>
>
>
commit 2f54f7ea786744540d9176cecc086cfbf32e7695
Author: Pavel Stehule <pavel.steh...@gooddata.com>
Date: Sun Jul 19 20:30:32 2015 +0200
initial
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index 7467e86..423757d 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -493,6 +493,22 @@ PostgreSQL documentation
</varlistentry>
<varlistentry>
+ <term><option>--strict-names</></term>
+ <listitem>
+ <para>
+ Require that table and/or schema without wildcard chars 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>--table=<replaceable class="parameter">table</replaceable></option></term>
<listitem>
diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
index d7506e1..598df0b 100644
--- a/src/bin/pg_dump/dumputils.c
+++ b/src/bin/pg_dump/dumputils.c
@@ -955,9 +955,9 @@ AddAcl(PQExpBuffer aclbuf, const char *keyword, const char *subname)
/*
- * processSQLNamePattern
+ * processSQLName
*
- * Scan a wildcard-pattern string and generate appropriate WHERE clauses
+ * Scan a possible wildcard-pattern string and generate appropriate WHERE clauses
* to limit the set of objects returned. The WHERE clauses are appended
* to the already-partially-constructed query in buf. Returns whether
* any clause was added.
@@ -978,12 +978,16 @@ AddAcl(PQExpBuffer aclbuf, const char *keyword, const char *subname)
*
* Formatting note: the text already present in buf should end with a newline.
* The appended text, if any, will end with one too.
+ *
+ * found_wildcard_char: pointer to boolean pointer, it is true, when name
+ * contains some wildcard char.
*/
bool
-processSQLNamePattern(PGconn *conn, PQExpBuffer buf, const char *pattern,
+processSQLName(PGconn *conn, PQExpBuffer buf, const char *pattern,
bool have_where, bool force_escape,
const char *schemavar, const char *namevar,
- const char *altnamevar, const char *visibilityrule)
+ const char *altnamevar, const char *visibilityrule,
+ bool *found_wildcard_char)
{
PQExpBufferData schemabuf;
PQExpBufferData namebuf;
@@ -993,6 +997,8 @@ processSQLNamePattern(PGconn *conn, PQExpBuffer buf, const char *pattern,
int i;
bool added_clause = false;
+ *found_wildcard_char = false;
+
#define WHEREAND() \
(appendPQExpBufferStr(buf, have_where ? " AND " : "WHERE "), \
have_where = true, added_clause = true)
@@ -1054,11 +1060,13 @@ processSQLNamePattern(PGconn *conn, PQExpBuffer buf, const char *pattern,
else if (!inquotes && ch == '*')
{
appendPQExpBufferStr(&namebuf, ".*");
+ *found_wildcard_char = true;
cp++;
}
else if (!inquotes && ch == '?')
{
appendPQExpBufferChar(&namebuf, '.');
+ *found_wildcard_char = true;
cp++;
}
else if (!inquotes && ch == '.')
@@ -1167,6 +1175,25 @@ processSQLNamePattern(PGconn *conn, PQExpBuffer buf, const char *pattern,
}
/*
+ * processSQLNamePattern
+ *
+ * Same as processSQLName but there are no important fact, if some wildcard is used or not.
+ */
+bool
+processSQLNamePattern(PGconn *conn, PQExpBuffer buf, const char *pattern,
+ bool have_where, bool force_escape,
+ const char *schemavar, const char *namevar,
+ const char *altnamevar, const char *visibilityrule)
+{
+ bool found_wildcard_char;
+
+ return processSQLName(conn, buf, pattern,
+ have_where, force_escape,
+ schemavar, namevar,
+ altnamevar, visibilityrule, &found_wildcard_char);
+}
+
+/*
* buildShSecLabelQuery
*
* Build a query to retrieve security labels for a shared object.
diff --git a/src/bin/pg_dump/dumputils.h b/src/bin/pg_dump/dumputils.h
index b176746..cef2a8b 100644
--- a/src/bin/pg_dump/dumputils.h
+++ b/src/bin/pg_dump/dumputils.h
@@ -90,6 +90,11 @@ extern bool buildDefaultACLCommands(const char *type, const char *nspname,
const char *acls, const char *owner,
int remoteVersion,
PQExpBuffer sql);
+extern bool processSQLName(PGconn *conn, PQExpBuffer buf, const char *pattern,
+ bool have_where, bool force_escape,
+ const char *schemavar, const char *namevar,
+ const char *altnamevar, const char *visibilityrule,
+ bool *found_wildcard_char);
extern bool processSQLNamePattern(PGconn *conn, PQExpBuffer buf,
const char *pattern,
bool have_where, bool force_escape,
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index 80df8fc..4109039 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -178,6 +178,7 @@ typedef struct _dumpOptions
int outputNoTablespaces;
int use_setsessauth;
int enable_row_security;
+ int strict_names;
/* 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
index 0e036b8..24931e8 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -131,10 +131,12 @@ static void setup_connection(Archive *AH, DumpOptions *dopt,
static ArchiveFormat parseArchiveFormat(const char *format, ArchiveMode *mode);
static void expand_schema_name_patterns(Archive *fout,
SimpleStringList *patterns,
- SimpleOidList *oids);
+ SimpleOidList *oids,
+ bool strict_names);
static void expand_table_name_patterns(Archive *fout,
SimpleStringList *patterns,
- SimpleOidList *oids);
+ 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);
@@ -332,6 +334,7 @@ main(int argc, char **argv)
{"section", required_argument, NULL, 5},
{"serializable-deferrable", no_argument, &dopt.serializable_deferrable, 1},
{"snapshot", required_argument, NULL, 6},
+ {"strict-names", no_argument, &dopt.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},
@@ -684,27 +687,32 @@ main(int argc, char **argv)
if (schema_include_patterns.head != NULL)
{
expand_schema_name_patterns(fout, &schema_include_patterns,
- &schema_include_oids);
+ &schema_include_oids,
+ dopt.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);
+ &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);
+ &table_include_oids,
+ dopt.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);
+ &table_exclude_oids,
+ false);
expand_table_name_patterns(fout, &tabledata_exclude_patterns,
- &tabledata_exclude_oids);
+ &tabledata_exclude_oids,
+ false);
/* non-matching exclusion patterns aren't an error */
@@ -899,6 +907,8 @@ help(const char *progname)
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"));
@@ -1129,7 +1139,8 @@ parseArchiveFormat(const char *format, ArchiveMode *mode)
static void
expand_schema_name_patterns(Archive *fout,
SimpleStringList *patterns,
- SimpleOidList *oids)
+ SimpleOidList *oids,
+ bool strict_names)
{
PQExpBuffer query;
PGresult *res;
@@ -1145,28 +1156,37 @@ expand_schema_name_patterns(Archive *fout,
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.
+ * We use more queries with possible 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");
+ bool found_wildcard_char;
+
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);
+ processSQLName(GetConnection(fout), query, cell->val, false,
+ false, NULL, "n.nspname", NULL, NULL,
+ &found_wildcard_char);
- for (i = 0; i < PQntuples(res); i++)
- {
- simple_oid_list_append(oids, atooid(PQgetvalue(res, i, 0)));
+ res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
+ if (PQntuples(res) > 0)
+ {
+ for (i = 0; i < PQntuples(res); i++)
+ simple_oid_list_append(oids, atooid(PQgetvalue(res, i, 0)));
+ }
+ else
+ {
+ if (strict_names && !found_wildcard_char)
+ exit_horribly(NULL, "Schema \"%s\" not found.\n", cell->val);
+ }
+
+ PQclear(res);
+ resetPQExpBuffer(query);
}
- PQclear(res);
destroyPQExpBuffer(query);
}
@@ -1176,8 +1196,10 @@ expand_schema_name_patterns(Archive *fout,
*/
static void
expand_table_name_patterns(Archive *fout,
- SimpleStringList *patterns, SimpleOidList *oids)
+ SimpleStringList *patterns, SimpleOidList *oids,
+ bool strict_names)
{
+ PQExpBuffer unioned_query;
PQExpBuffer query;
PGresult *res;
SimpleStringListCell *cell;
@@ -1186,6 +1208,7 @@ expand_table_name_patterns(Archive *fout,
if (patterns->head == NULL)
return; /* nothing to do */
+ unioned_query = createPQExpBuffer();
query = createPQExpBuffer();
/*
@@ -1195,8 +1218,8 @@ expand_table_name_patterns(Archive *fout,
for (cell = patterns->head; cell; cell = cell->next)
{
- if (cell != patterns->head)
- appendPQExpBufferStr(query, "UNION ALL\n");
+ bool found_wildcard_char;
+
appendPQExpBuffer(query,
"SELECT c.oid"
"\nFROM pg_catalog.pg_class c"
@@ -1204,19 +1227,47 @@ expand_table_name_patterns(Archive *fout,
"\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,
+ processSQLName(GetConnection(fout), query, cell->val, true,
false, "n.nspname", "c.relname", NULL,
- "pg_catalog.pg_table_is_visible(c.oid)");
- }
+ "pg_catalog.pg_table_is_visible(c.oid)",
+ &found_wildcard_char);
- res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
+ if (strict_names && !found_wildcard_char)
+ {
+ res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
+
+ if (PQntuples(res) > 0)
+ {
+ for (i = 0; i < PQntuples(res); i++)
+ simple_oid_list_append(oids, atooid(PQgetvalue(res, i, 0)));
+ }
+ else
+ exit_horribly(NULL, "Table \"%s\" not found.\n", cell->val);
+
+ PQclear(res);
+ }
+ else
+ {
+ if (unioned_query->len > 0)
+ appendPQExpBufferStr(unioned_query, "UNION ALL\n");
+
+ appendPQExpBuffer(unioned_query, query->data, query->len);
+ }
+
+ resetPQExpBuffer(query);
+ }
- for (i = 0; i < PQntuples(res); i++)
+ if (unioned_query->len > 0)
{
- simple_oid_list_append(oids, atooid(PQgetvalue(res, i, 0)));
+ res = ExecuteSqlQuery(fout, unioned_query->data, PGRES_TUPLES_OK);
+
+ for (i = 0; i < PQntuples(res); i++)
+ simple_oid_list_append(oids, atooid(PQgetvalue(res, i, 0)));
+
+ PQclear(res);
}
- PQclear(res);
+ destroyPQExpBuffer(unioned_query);
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
