Re: [HACKERS] pg_dump quietly ignore missing tables - is it bug?

[Michael Paquier]

On Sun, Aug 23, 2015 at 10:47 PM, Pavel Stehule  wrote:
> [blah]
>
> fixed

Moved to next CF 2015-09.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_dump quietly ignore missing tables - is it bug?

[Pavel Stehule]

Hi

2015-08-22 0:09 GMT+02:00 Jim Nasby :

> The following review has been posted through the commitfest application:
> make installcheck-world:  tested, passed
> Implements feature:   tested, failed
> Spec compliant:   not tested
> Documentation:not tested
>
> The feature doesn't seem to work:
> pg_dump -t t -t 'ii*' --strict-names
> pg_dump: unrecognized option `--strict-names'
> Try "pg_dump --help" for more information.
> decibel@decina:[16:58]~/git/postgres/i
> (pg_dump-strict-names-7.patch=)$bin/p
>

sorry - there was wrong strict-mode

fixed


>
> The documentation could use some improvements.
>
> +
> + 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.
> +
>
> Would be clearer as
>
> Require that each schema (-n / --schema) and table (-t / --table)
> qualifier match at least one schema/table in the database to be dumped.
> Note that if none of the schema/table qualifiers find matches pg_dump will
> generate an error even without --strict-names.
>
> +
> + This option has no effect on the exclude table and schema
> patterns
> + (and also --exclude-table-data): not matching any
> entities
> + isn't considered an error.
>
> Rewrite:
> This option has no effect on -N/--exclude-schema, -T/--exclude_table or
> --exclude-table-date. An exclude pattern failing to match any objects is
> not considered an error.
>
>
fixed

Regards

Pavel



> The new status of this patch is: Waiting on Author
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index 7467e86..eaa006d 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -545,6 +545,23 @@ PostgreSQL documentation
  
 
  
+  --strict-names
+  
+   
+Require that each schema (-n / --schema) and table (-t / --table)
+qualifier match at least one schema/table in the database to be dumped.
+Note that if none of the schema/table qualifiers find matches pg_dump
+will generate an error even without --strict-names.
+   
+   
+This option has no effect on -N/--exclude-schema, -T/--exclude_table
+or --exclude-table-date. An exclude pattern failing to match
+any bjects is not considered an error.
+   
+  
+ 
+
+ 
   -T table
   --exclude-table=table
   
diff --git a/doc/src/sgml/ref/pg_restore.sgml b/doc/src/sgml/ref/pg_restore.sgml
index 97e3420..a5a9394 100644
--- a/doc/src/sgml/ref/pg_restore.sgml
+++ b/doc/src/sgml/ref/pg_restore.sgml
@@ -432,6 +432,16 @@
  
 
  
+  --strict-names
+  
+   
+Require that each schema (-n / --schema) and table (-t / --table)
+qualifier match at least one schema/table in the backup file.
+   
+  
+ 
+
+ 
   -T trigger
   --trigger=trigger
   
diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
index d7506e1..52b2b98 100644
--- a/src/bin/pg_dump/dumputils.c
+++ b/src/bin/pg_dump/dumputils.c
@@ -1220,6 +1220,7 @@ simple_string_list_append(SimpleStringList *list, const char *val)
 		pg_malloc(offsetof(SimpleStringListCell, val) +strlen(val) + 1);
 
 	cell->next = NULL;
+	cell->touched = false;
 	strcpy(cell->val, val);
 
 	if (list->tail)
@@ -1237,7 +1238,23 @@ simple_string_list_member(SimpleStringList *list, const char *val)
 	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
index b176746..9f31bbc 100644
--- a/src/bin/pg_dump/dumputils.h
+++ b/src/bin/pg_dump/dumputils.h
@@ -38,6 +38,8 @@ typedef struct SimpleOidList
 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;
 
@@ -103,5 +105,7 @@ extern void set_dump_section(const char *arg, int *dumpSections);
 
 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
index 80df8fc..

Re: [HACKERS] pg_dump quietly ignore missing tables - is it bug?

[Jim Nasby]

The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:   tested, failed
Spec compliant:   not tested
Documentation:not tested

The feature doesn't seem to work:
pg_dump -t t -t 'ii*' --strict-names
pg_dump: unrecognized option `--strict-names'
Try "pg_dump --help" for more information.
decibel@decina:[16:58]~/git/postgres/i (pg_dump-strict-names-7.patch=)$bin/p

The documentation could use some improvements.

+
+ 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.
+

Would be clearer as

Require that each schema (-n / --schema) and table (-t / --table) qualifier 
match at least one schema/table in the database to be dumped. Note that if none 
of the schema/table qualifiers find matches pg_dump will generate an error even 
without --strict-names.

+
+ This option has no effect on the exclude table and schema patterns
+ (and also --exclude-table-data): not matching any entities
+ isn't considered an error.

Rewrite:
This option has no effect on -N/--exclude-schema, -T/--exclude_table or 
--exclude-table-date. An exclude pattern failing to match any objects is not 
considered an error.

The new status of this patch is: Waiting on Author


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_dump quietly ignore missing tables - is it bug?

[Pavel Stehule]

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 :

> Hi
>
> 2015-07-30 12:44 GMT+02:00 Heikki Linnakangas :
>
>> 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 
   
  
   
+   --strict-names
+   
+
+ 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.
+
+
+ This option has no effect on the exclude table and schema patterns
+ (and also --exclude-table-data): not matching any entities
+ isn't considered an error.
+
+   
+  
+ 
+  
-T table
--exclude-table=table

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 
   
  
   
+   --strict-names
+   
+
+ 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.
+
+   
+  
+ 
+  
-T trigger
--trigger=trigger

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_

Re: [HACKERS] pg_dump quietly ignore missing tables - is it bug?

[Pavel Stehule]

Hi

2015-07-30 12:44 GMT+02:00 Heikki Linnakangas :

> 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
>
>

Re: [HACKERS] pg_dump quietly ignore missing tables - is it bug?

[Heikki Linnakangas]

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.


For symmetry, it would be good to also support this option in 
pg_restore. It seems even more useful there.


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.


- Heikki



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_dump quietly ignore missing tables - is it bug?

[Pavel Stehule]

Hi

I am sending a new patch - without checking wildcard chars.

Regards

Pavel

2015-07-23 7:22 GMT+02:00 Kyotaro HORIGUCHI :

> Hello,
>
> > > 2015-07-19 20:54 GMT+02:00 Pavel Stehule :
> > >> 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).
>
> I share the same option with Tom that it should behave in same
> way regardless of the appearance of wildcards.
>
> You replied Tom as this
>
> > the behave is same - only one real identifier is allowed
>
> But the description above about this patch apparently says that
> they are differently handled. And
> expand_(schema|table)_name_patterns does the further differently
> thing from both of Tom's suggestion and what mentioned in your
> reply to that. I will mention for this topic again in this mail.
>
> # Might "only one real ident is allowed" mean "at most one
> # match", but not "exactly one match"?
>
> They do like this when strict-names.
>
>   - Not allow no match for non-wildcarded names.
>
>   - Allow no match for any wildcarded name spec and finally
> allowing *all* of them don't match anyting.
>
> This looks to me quite confusing.
>
> > >>  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).
>
> The new name "processSQLName" looks a bit
> bogus. processSQLNameIntenral would be a name commonly seen in
> such cases.
>
> > >> 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.
>
> Why do you think so? Wild cards are usually used to glob multiple
> names at once. One or more matches are expected for many or
> perhaps most cases, I think. Since so, if someone anticipate that
> some of his patterns have no match, I think he shouldn't specify
> --strict-names option at all.
>
> Furthermore, I don't think no one wants to use both wildcarded
> and non-wildcarded name specs at once but this is a little out of
> scope.
>
> I'd like to have opinions from others about this point.
>
> > > other variant is using --strict-names behave as default (and implement
> > > negative option like --disable-strict-names or some similar).
>
> This contradicts Josh's request. (which I'm not totally agree:p)
>
> > Note: originally I though, we have to fix it and change the default
> behave.
> > But with special option, we don't need it. This option in help is signal
> > for user, so some is risky.
>
> regards,
>
>
> --
> Kyotaro Horiguchi
> NTT Open Source Software Center
>
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 
   
  
   
+   --strict-names
+   
+
+ 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.
+
+
+ This option has no effect on the exclude table and schema patterns
+ (and also --exclude-table-data): not matching any entities
+ isn't considered an error.
+
+   
+  
+ 
+  
-T table
--exclude-table=table

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
new file mode 100644
index 0e036b8..54618fa
*** 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_mode = 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 

Re: [HACKERS] pg_dump quietly ignore missing tables - is it bug?

[Kyotaro HORIGUCHI]

Sorry for the bogus on bogus.

At Thu, 23 Jul 2015 14:22:59 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI 
 wrote in 
<20150723.142259.200902861.horiguchi.kyot...@lab.ntt.co.jp>
> Hello,
> 
> > > 2015-07-19 20:54 GMT+02:00 Pavel Stehule :
> > >> 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).
> 
> I share the same option with Tom that it should behave in same
> way regardless of the appearance of wildcards.
> 
> You replied Tom as this
> 
> > the behave is same - only one real identifier is allowed
> 
> But the description above about this patch apparently says that
> they are differently handled. And
> expand_(schema|table)_name_patterns does the further differently
> thing from both of Tom's suggestion and what mentioned in your
> reply to that. I will mention for this topic again in this mail.
> 
> # Might "only one real ident is allowed" mean "at most one
> # match", but not "exactly one match"?
> 
> They do like this when strict-names.
> 
>   - Not allow no match for non-wildcarded names.
> 
>   - Allow no match for any wildcarded name spec and finally
> allowing *all* of them don't match anyting.
> 
> This looks to me quite confusing.
> 
> > >>  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).
> 
> The new name "processSQLName" looks a bit
> bogus. processSQLNameIntenral would be a name commonly seen in
> such cases.

The ocrrent name is not that but processSQLNamePatternInternal.

> > >> 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.
> 
> Why do you think so? Wild cards are usually used to glob multiple
> names at once. One or more matches are expected for many or
> perhaps most cases, I think. Since so, if someone anticipate that
> some of his patterns have no match, I think he shouldn't specify
> --strict-names option at all.
> 
> Furthermore, I don't think no one wants to use both wildcarded
> and non-wildcarded name specs at once but this is a little out of
> scope.
> 
> I'd like to have opinions from others about this point.
> 
> > > other variant is using --strict-names behave as default (and implement
> > > negative option like --disable-strict-names or some similar).
> 
> This contradicts Josh's request. (which I'm not totally agree:p)
> 
> > Note: originally I though, we have to fix it and change the default behave.
> > But with special option, we don't need it. This option in help is signal
> > for user, so some is risky.
> 
> regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_dump quietly ignore missing tables - is it bug?

[Kyotaro HORIGUCHI]

Hello,

> > 2015-07-19 20:54 GMT+02:00 Pavel Stehule :
> >> 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).

I share the same option with Tom that it should behave in same
way regardless of the appearance of wildcards.

You replied Tom as this

> the behave is same - only one real identifier is allowed

But the description above about this patch apparently says that
they are differently handled. And
expand_(schema|table)_name_patterns does the further differently
thing from both of Tom's suggestion and what mentioned in your
reply to that. I will mention for this topic again in this mail.

# Might "only one real ident is allowed" mean "at most one
# match", but not "exactly one match"?

They do like this when strict-names.

  - Not allow no match for non-wildcarded names.

  - Allow no match for any wildcarded name spec and finally
allowing *all* of them don't match anyting.

This looks to me quite confusing.

> >>  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).

The new name "processSQLName" looks a bit
bogus. processSQLNameIntenral would be a name commonly seen in
such cases.

> >> 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.

Why do you think so? Wild cards are usually used to glob multiple
names at once. One or more matches are expected for many or
perhaps most cases, I think. Since so, if someone anticipate that
some of his patterns have no match, I think he shouldn't specify
--strict-names option at all.

Furthermore, I don't think no one wants to use both wildcarded
and non-wildcarded name specs at once but this is a little out of
scope.

I'd like to have opinions from others about this point.

> > other variant is using --strict-names behave as default (and implement
> > negative option like --disable-strict-names or some similar).

This contradicts Josh's request. (which I'm not totally agree:p)

> Note: originally I though, we have to fix it and change the default behave.
> But with special option, we don't need it. This option in help is signal
> for user, so some is risky.

regards,


-- 
Kyotaro Horiguchi
NTT Open Source Software Center


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_dump quietly ignore missing tables - is it bug?

[Pavel Stehule]

2015-07-19 21:08 GMT+02:00 Pavel Stehule :

>
>
> 2015-07-19 20:54 GMT+02:00 Pavel Stehule :
>
>> 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.
>>
>
> other variant is using --strict-names behave as default (and implement
> negative option like --disable-strict-names or some similar).
>

Note: originally I though, we have to fix it and change the default behave.
But with special option, we don't need it. This option in help is signal
for user, so some is risky.

Pavel

>
>
>
>>
>> Regards
>>
>> Pavel
>>
>>
>>
>>
>>
>>
>> 2015-07-09 22:48 GMT+02:00 Pavel Stehule :
>>
>>>
>>>
>>> 2015-07-08 5:36 GMT+02:00 Fujii Masao :
>>>
 On Sat, May 23, 2015 at 1:41 AM, Pavel Stehule 
 wrote:
 >
 >
 > 2015-05-22 18:34 GMT+02:00 Tom Lane :
 >>
 >> Oleksandr Shulgin  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

>>>
>>>
>>
>

Re: [HACKERS] pg_dump quietly ignore missing tables - is it bug?

[Pavel Stehule]

2015-07-19 20:54 GMT+02:00 Pavel Stehule :

> 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.
>

other variant is using --strict-names behave as default (and implement
negative option like --disable-strict-names or some similar).



>
> Regards
>
> Pavel
>
>
>
>
>
>
> 2015-07-09 22:48 GMT+02:00 Pavel Stehule :
>
>>
>>
>> 2015-07-08 5:36 GMT+02:00 Fujii Masao :
>>
>>> On Sat, May 23, 2015 at 1:41 AM, Pavel Stehule 
>>> wrote:
>>> >
>>> >
>>> > 2015-05-22 18:34 GMT+02:00 Tom Lane :
>>> >>
>>> >> Oleksandr Shulgin  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
>>>
>>
>>
>

Re: [HACKERS] pg_dump quietly ignore missing tables - is it bug?

[Pavel Stehule]

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 :

>
>
> 2015-07-08 5:36 GMT+02:00 Fujii Masao :
>
>> On Sat, May 23, 2015 at 1:41 AM, Pavel Stehule 
>> wrote:
>> >
>> >
>> > 2015-05-22 18:34 GMT+02:00 Tom Lane :
>> >>
>> >> Oleksandr Shulgin  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 
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
  
 
  
+  --strict-names
+  
+   
+ 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.
+   
+   
+ This option has no effect on the exclude table and schema patterns
+ (and also --exclude-table-data): not matching any entities
+ isn't considered an error.
+   
+  
+ 
+
+ 
   -t table
   --table=table
   
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 WHE

Re: [HACKERS] pg_dump quietly ignore missing tables - is it bug?

[Pavel Stehule]

2015-07-08 5:36 GMT+02:00 Fujii Masao :

> On Sat, May 23, 2015 at 1:41 AM, Pavel Stehule 
> wrote:
> >
> >
> > 2015-05-22 18:34 GMT+02:00 Tom Lane :
> >>
> >> Oleksandr Shulgin  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
>

Re: [HACKERS] pg_dump quietly ignore missing tables - is it bug?

[Fujii Masao]

On Sat, May 23, 2015 at 1:41 AM, Pavel Stehule  wrote:
>
>
> 2015-05-22 18:34 GMT+02:00 Tom Lane :
>>
>> Oleksandr Shulgin  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".

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.

Isn't it sufficient to only emit the warning message to stderr if there
is no table matching the pattern specified in -t?

Regards,

-- 
Fujii Masao


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_dump quietly ignore missing tables - is it bug?

[Shulgin, Oleksandr]

On Fri, May 22, 2015 at 6:34 PM, Tom Lane  wrote:

> Oleksandr Shulgin  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.
>

My patch does that, in the case of no-wildcards -t argument.

However, it can be fixed easily: just drop that strcspn() call, and then
default behavior is the same for both wildcard and exact matches, since
--strict-include is off by default.

--
Alex

Re: [HACKERS] pg_dump quietly ignore missing tables - is it bug?

[Pavel Stehule]

2015-05-22 18:34 GMT+02:00 Tom Lane :

> Oleksandr Shulgin  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"

Pavel


>
> regards, tom lane
>

Re: [HACKERS] pg_dump quietly ignore missing tables - is it bug?

[Pavel Stehule]

2015-05-22 18:35 GMT+02:00 Shulgin, Oleksandr 
:

> On Fri, May 22, 2015 at 6:32 PM, Pavel Stehule 
> wrote:
>
>>
>>
>> 2015-05-22 18:30 GMT+02:00 Shulgin, Oleksandr <
>> oleksandr.shul...@zalando.de>:
>>
>>> On Fri, May 22, 2015 at 6:09 PM, Pavel Stehule 
>>> wrote:
>>>

 2015-05-21 16:48 GMT+02:00 Oleksandr Shulgin <
 oleksandr.shul...@zalando.de>:

>
> 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).
>

 it was prototype - I believe so issue with describe.c can be solved
 better


>
> 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).
>

 hard to say - any variant has own advantages and disadvantages

 But I more to unlike it than like - it is more usual, when you use
 exact name so, you need it exactly one, and when you use some wildcard, so
 you are expecting one or more tables.

 This use case is not checked in your patch.

>>>
>>> Maybe I'm missing something, but I believe it's handled by
>>>
>>> pg_dump -t mytables* --strict-include
>>>
>>> so that it will error out if nothing was found for mytables* pattern.
>>>
>>
>> If I understand it raise a error when it found more than one table
>>
>
> I hope not, and that totally was not my intent :-p
>
> It should raise if it found *less than* one, that is: none.
>

ok, then I have not objection

Re: [HACKERS] pg_dump quietly ignore missing tables - is it bug?

[Shulgin, Oleksandr]

On Fri, May 22, 2015 at 6:32 PM, Pavel Stehule 
wrote:

>
>
> 2015-05-22 18:30 GMT+02:00 Shulgin, Oleksandr <
> oleksandr.shul...@zalando.de>:
>
>> On Fri, May 22, 2015 at 6:09 PM, Pavel Stehule 
>> wrote:
>>
>>>
>>> 2015-05-21 16:48 GMT+02:00 Oleksandr Shulgin <
>>> oleksandr.shul...@zalando.de>:
>>>

 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).

>>>
>>> it was prototype - I believe so issue with describe.c can be solved
>>> better
>>>
>>>

 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).

>>>
>>> hard to say - any variant has own advantages and disadvantages
>>>
>>> But I more to unlike it than like - it is more usual, when you use exact
>>> name so, you need it exactly one, and when you use some wildcard, so you
>>> are expecting one or more tables.
>>>
>>> This use case is not checked in your patch.
>>>
>>
>> Maybe I'm missing something, but I believe it's handled by
>>
>> pg_dump -t mytables* --strict-include
>>
>> so that it will error out if nothing was found for mytables* pattern.
>>
>
> If I understand it raise a error when it found more than one table
>

I hope not, and that totally was not my intent :-p

It should raise if it found *less than* one, that is: none.

Re: [HACKERS] pg_dump quietly ignore missing tables - is it bug?

[Tom Lane]

Oleksandr Shulgin  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.)

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_dump quietly ignore missing tables - is it bug?

[Pavel Stehule]

2015-05-22 18:30 GMT+02:00 Shulgin, Oleksandr 
:

> On Fri, May 22, 2015 at 6:09 PM, Pavel Stehule 
> wrote:
>
>>
>> 2015-05-21 16:48 GMT+02:00 Oleksandr Shulgin <
>> oleksandr.shul...@zalando.de>:
>>
>>>
>>> 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).
>>>
>>
>> it was prototype - I believe so issue with describe.c can be solved better
>>
>>
>>>
>>> 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).
>>>
>>
>> hard to say - any variant has own advantages and disadvantages
>>
>> But I more to unlike it than like - it is more usual, when you use exact
>> name so, you need it exactly one, and when you use some wildcard, so you
>> are expecting one or more tables.
>>
>> This use case is not checked in your patch.
>>
>
> Maybe I'm missing something, but I believe it's handled by
>
> pg_dump -t mytables* --strict-include
>
> so that it will error out if nothing was found for mytables* pattern.
>

If I understand it raise a error when it found more than one table

Pavel


>
> --
> Alex
>
>

Re: [HACKERS] pg_dump quietly ignore missing tables - is it bug?

[Shulgin, Oleksandr]

On Fri, May 22, 2015 at 6:09 PM, Pavel Stehule 
wrote:

>
> 2015-05-21 16:48 GMT+02:00 Oleksandr Shulgin  >:
>
>>
>> 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).
>>
>
> it was prototype - I believe so issue with describe.c can be solved better
>
>
>>
>> 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).
>>
>
> hard to say - any variant has own advantages and disadvantages
>
> But I more to unlike it than like - it is more usual, when you use exact
> name so, you need it exactly one, and when you use some wildcard, so you
> are expecting one or more tables.
>
> This use case is not checked in your patch.
>

Maybe I'm missing something, but I believe it's handled by

pg_dump -t mytables* --strict-include

so that it will error out if nothing was found for mytables* pattern.

--
Alex

Re: [HACKERS] pg_dump quietly ignore missing tables - is it bug?

[Pavel Stehule]

2015-05-21 16:48 GMT+02:00 Oleksandr Shulgin :

> Pavel Stehule  writes:
> >
> > 2015-03-23 17:11 GMT+01:00 Pavel Stehule :
> >
> >> Hi
> >>
> >> 2015-03-15 16:09 GMT+01:00 Tom Lane :
> >>
> >>> Pavel Stehule  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).
>

it was prototype - I believe so issue with describe.c can be solved better


>
> 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).
>

hard to say - any variant has own advantages and disadvantages

But I more to unlike it than like - it is more usual, when you use exact
name so, you need it exactly one, and when you use some wildcard, so you
are expecting one or more tables.

This use case is not checked in your patch.

Regards

Pavel


>
> Please see attached patch.
>
> --
> Alex
>
>

Re: [HACKERS] pg_dump quietly ignore missing tables - is it bug?

[Oleksandr Shulgin]

Pavel Stehule  writes:
>
> 2015-03-23 17:11 GMT+01:00 Pavel Stehule :
>
>> Hi
>>
>> 2015-03-15 16:09 GMT+01:00 Tom Lane :
>>
>>> Pavel Stehule  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..da1
*** a/doc/src/sgml/ref/pg_dump.sgml
--- b/doc/src/sgml/ref/pg_dump.sgml
*** PostgreSQL documentation
*** 365,370 
--- 365,377 
  .
 
  
+
+  When schema 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 --strict-include option.
+
+ 
 
  
   When -n is specified, pg_dump
*** PostgreSQL documentation
*** 515,520 
--- 522,534 
 
  
 
+  When table 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 --strict-include option.
+
+ 
+
  The -n and -N switches have no effect when
  -t is used, because tables selected by -t will
  be dumped regardless of those switches, and non-table objects will not
*** PostgreSQL documentation
*** 902,907 
--- 916,938 
 

   
+ 
+  
+   --strict-include
+   
+
+  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.
+
+
+  This option has no effect on the exclude table and schema patterns
+  (and also --exclude-table-data): not matching any entities
+  isn't considered an error.
+
+   
+  
  
   
--use-set-session-authorization
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 exp

Re: [HACKERS] pg_dump quietly ignore missing tables - is it bug?

[Pavel Stehule]

2015-03-23 17:11 GMT+01:00 Pavel Stehule :

> Hi
>
> 2015-03-15 16:09 GMT+01:00 Tom Lane :
>
>> Pavel Stehule  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

Regards

Pavel


>
> Regards
>
> Pavel
>
>
>>
>> Neither of those is what you wrote, though.
>>
>> If we implemented the second one of these, it would have to be controlled
>> by a new switch, because there are plausible use cases for wildcards that
>> sometimes don't match anything (not to mention backwards compatibility).
>> There might be a reasonable argument for the first one being the
>> default behavior, though; I'm not sure if we could get away with that
>> from a compatibility perspective.
>>
>> regards, tom lane
>>
>
>
diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
new file mode 100644
index d7506e1..47ae6b8
*** a/src/bin/pg_dump/dumputils.c
--- b/src/bin/pg_dump/dumputils.c
*** bool
*** 983,989 
  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)
  {
  	PQExpBufferData schemabuf;
  	PQExpBufferData namebuf;
--- 983,990 
  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 *with_wildcards)
  {
  	PQExpBufferData schemabuf;
  	PQExpBufferData namebuf;
*** processSQLNamePattern(PGconn *conn, PQEx
*** 997,1002 
--- 998,1007 
  	(appendPQExpBufferStr(buf, have_where ? "  AND " : "WHERE "), \
  	 have_where = true, added_clause = true)
  
+ #define SET_WITH_WILDCARDS(b)	if (with_wildcards) *with_wildcards = b;
+ 
+ 	SET_WITH_WILDCARDS(false);
+ 
  	if (pattern == NULL)
  	{
  		/* Default: select all visible objects */
*** processSQLNamePattern(PGconn *conn, PQEx
*** 1055,1065 
--- 1060,1074 
  		{
  			appendPQExpBufferStr(&namebuf, ".*");
  			cp++;
+ 
+ 			SET_WITH_WILDCARDS(true);
  		}
  		else if (!inquotes && ch == '?')
  		{
  			appendPQExpBufferChar(&namebuf, '.');
  			cp++;
+ 
+ 			SET_WITH_WILDCARDS(true);
  		}
  		else if (!inquotes && ch == '.')
  		{
diff --git a/src/bin/pg_dump/dumputils.h b/src/bin/pg_dump/dumputils.h
new file mode 100644
index b176746..7fb7b62
*** a/src/bin/pg_dump/dumputils.h
--- b/src/bin/pg_dump/dumputils.h
*** extern bool processSQLNamePattern(PGconn
*** 94,100 
  	  const char *pattern,
  	  bool have_where, bool force_escape,
  	  const char *schemavar, const char *namevar,
! 	  const char *altnamevar, const char *visibilityrule);
  extern void buildShSecLabelQuery(PGconn *conn, const char *catalog_name,
  	 uint32 objectId, PQExpBuffer sql);
  extern void emitShSecLabels(PGconn *conn, PGresult *res,
--- 94,101 
  	  const char *pattern,
  	  bool have_where, bool force_escape,
  	  const char *schemavar, const char *namevar,
! 	  const char *altnamevar, const char *visibilityrule,
! 	  bool *with_wildcards);
  extern void buildShSecLabelQuery(PGconn *conn, const char *catalog_name,
  	 uint32 objectId, PQExpBuffer sql);
  extern void emitShSecLabels(PGconn *conn, PGresult *res,
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
new file mode 100644
index f24fefa..ff1e6a0
*** a/src/bin/pg_dump/pg_dump.c
--- b/src/bin/pg_dump/pg_dump.c
*** static SimpleStringList schema_include_p
*** 107,112 
--- 107,113 
  static SimpleOidList schema_include_oids = {NULL, NULL};
  static SimpleStringList schema_exclude_patterns = {NULL, NULL};
  static SimpleOidList schema_exclude_oids = {NULL, NULL};
+ static SimpleStringList schema_optional_patterns = {NULL, NULL};
  
  static SimpleStringList table_include_patterns = {NULL, NULL};
  static SimpleOidList table_include_oids = {NULL, NULL};
*** static SimpleStringList table_exclude_pa
*** 114,119 
--- 115,121 
  static SimpleOidList table_exclude_oids = {NULL, NULL};
  static SimpleStringLi

Re: [HACKERS] pg_dump quietly ignore missing tables - is it bug?

[Pavel Stehule]

Hi

2015-03-15 16:09 GMT+01:00 Tom Lane :

> Pavel Stehule  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

Regards

Pavel


>
> Neither of those is what you wrote, though.
>
> If we implemented the second one of these, it would have to be controlled
> by a new switch, because there are plausible use cases for wildcards that
> sometimes don't match anything (not to mention backwards compatibility).
> There might be a reasonable argument for the first one being the
> default behavior, though; I'm not sure if we could get away with that
> from a compatibility perspective.
>
> regards, tom lane
>
commit c1c7d9671a751bda1918d479b81c38c538701ec1
Author: Pavel Stehule 
Date:   Mon Mar 23 17:06:39 2015 +0100

initial

diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
index d7506e1..47ae6b8 100644
--- a/src/bin/pg_dump/dumputils.c
+++ b/src/bin/pg_dump/dumputils.c
@@ -983,7 +983,8 @@ 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)
+	  const char *altnamevar, const char *visibilityrule,
+	  bool *with_wildcards)
 {
 	PQExpBufferData schemabuf;
 	PQExpBufferData namebuf;
@@ -997,6 +998,10 @@ processSQLNamePattern(PGconn *conn, PQExpBuffer buf, const char *pattern,
 	(appendPQExpBufferStr(buf, have_where ? "  AND " : "WHERE "), \
 	 have_where = true, added_clause = true)
 
+#define SET_WITH_WILDCARDS(b)	if (with_wildcards) *with_wildcards = b;
+
+	SET_WITH_WILDCARDS(false);
+
 	if (pattern == NULL)
 	{
 		/* Default: select all visible objects */
@@ -1055,11 +1060,15 @@ processSQLNamePattern(PGconn *conn, PQExpBuffer buf, const char *pattern,
 		{
 			appendPQExpBufferStr(&namebuf, ".*");
 			cp++;
+
+			SET_WITH_WILDCARDS(true);
 		}
 		else if (!inquotes && ch == '?')
 		{
 			appendPQExpBufferChar(&namebuf, '.');
 			cp++;
+
+			SET_WITH_WILDCARDS(true);
 		}
 		else if (!inquotes && ch == '.')
 		{
diff --git a/src/bin/pg_dump/dumputils.h b/src/bin/pg_dump/dumputils.h
index b176746..7fb7b62 100644
--- a/src/bin/pg_dump/dumputils.h
+++ b/src/bin/pg_dump/dumputils.h
@@ -94,7 +94,8 @@ extern 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);
+	  const char *altnamevar, const char *visibilityrule,
+	  bool *with_wildcards);
 extern void buildShSecLabelQuery(PGconn *conn, const char *catalog_name,
 	 uint32 objectId, PQExpBuffer sql);
 extern void emitShSecLabels(PGconn *conn, PGresult *res,
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index f24fefa..dd1d813 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -114,6 +114,7 @@ static SimpleStringList table_exclude_patterns = {NULL, NULL};
 static SimpleOidList table_exclude_oids = {NULL, NULL};
 static SimpleStringList tabledata_exclude_patterns = {NULL, NULL};
 static SimpleOidList tabledata_exclude_oids = {NULL, NULL};
+static SimpleStringList optional_tables = {NULL, NULL};
 
 
 char		g_opaque_type[10];	/* name for the opaque type */
@@ -132,7 +133,7 @@ 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,
+static void expand_table_name_patterns(Archive *fout, bool check_patterns,
 		   SimpleStringList *patterns,
 		   SimpleOidList *oids);
 static NamespaceInfo *findNamespace(Archive *fout, Oid nsoid, Oid objoid);
@@ -332,6 +333,7 @@ main(int argc, char **argv)
 		{"section", required_argument, NULL, 5},
 		{"serializable-deferrable", no_argument, &dopt.serializable_deferrable, 1},
 		{"snapshot", required_argument, NULL, 6},
+		{"table-if-exists", required_argument, NULL, 7},
 		{"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_s

Re: [HACKERS] pg_dump quietly ignore missing tables - is it bug?

[Pavel Stehule]

2015-03-15 16:09 GMT+01:00 Tom Lane :

> Pavel Stehule  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?
>

the behave is same - only one real identifier is allowed


>
> 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.
>
> Neither of those is what you wrote, though.
>
> If we implemented the second one of these, it would have to be controlled
> by a new switch, because there are plausible use cases for wildcards that
> sometimes don't match anything (not to mention backwards compatibility).
> There might be a reasonable argument for the first one being the
> default behavior, though; I'm not sure if we could get away with that
> from a compatibility perspective.
>

both your variant has sense for me. We can implement these points
separately. And I see a first point as much more important than second.
Because there is a significant risk of hidden broken backup.

We can implement a some long option with same functionality like now - for
somebody who need backup some explicitly specified tables optionally. Maybe
"--table-if-exists" ??

Is it acceptable for you?

Regards

Pavel








>
> regards, tom lane
>

Re: [HACKERS] pg_dump quietly ignore missing tables - is it bug?

[Tom Lane]

Pavel Stehule  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.

Neither of those is what you wrote, though.

If we implemented the second one of these, it would have to be controlled
by a new switch, because there are plausible use cases for wildcards that
sometimes don't match anything (not to mention backwards compatibility).
There might be a reasonable argument for the first one being the
default behavior, though; I'm not sure if we could get away with that
from a compatibility perspective.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_dump quietly ignore missing tables - is it bug?

[Pavel Stehule]

2015-03-14 19:33 GMT+01:00 Pavel Stehule :

>
>
> 2015-03-13 23:43 GMT+01:00 Josh Berkus :
>
>> On 03/13/2015 10:01 AM, Pavel Stehule wrote:
>> >
>> >
>> > 2015-03-13 17:39 GMT+01:00 Robert Haas > > >:
>> >
>> > On Fri, Mar 13, 2015 at 11:26 AM, Pavel Stehule
>> > mailto:pavel.steh...@gmail.com>> wrote:
>> > > we found possible bug in pg_dump. It raise a error only when all
>> > specified
>> > > tables doesn't exists. When it find any table, then ignore missing
>> > other.
>> > >
>> > > /usr/local/pgsql/bin/pg_dump -t Foo -t omega -s postgres >
>> > /dev/null; echo
>> > > $?
>> > >
>> > > foo doesn't exists - it creates broken backup due missing "Foo"
>> table
>> > >
>> > >  [pavel@localhost include]$ /usr/local/pgsql/bin/pg_dump -t Foo
>> -t
>> > omegaa -s
>> > > postgres > /dev/null; echo $?
>> > > pg_dump: No matching tables were found
>> > > 1
>> > >
>> > > Is it ok? I am thinking, so it is potentially dangerous. Any
>> > explicitly
>> > > specified table should to exists.
>> >
>> > Keep in mind that the argument to -t is a pattern, not just a table
>> > name.  I'm not sure how much that affects the calculus here, but
>> it's
>> > something to think about.
>> >
>> >
>> > yes, it has a sense, although now, I am don't think so it was a good
>> > idea. There should be some difference between table name and table
>> pattern.
>>
>> There was a long discussion about this when the feature was introduced
>> in 7.3(?) IIRC.  Changing it now would break backwards-compatibility, so
>> you'd really need to introduce a new option.
>>
>> And, if you introduce a new option which is a specific table name, would
>> you require a schema name or not?
>>
>
> We can use a same rules like we use for STRICT clause somewhere. There
> should be only one table specified by the option. If it is not necessary,
> then only name is enough, else qualified name is necessary.
>
> what is a name for this option? Maybe only with long name - some like
> 'required-table' ?
>

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.

Regards

Pavel


>
> Regards
>
> Pavel
>
>
>>
>> --
>> Josh Berkus
>> PostgreSQL Experts Inc.
>> http://pgexperts.com
>>
>
>
commit 3d3c6f6583c44a06633aea7978ad0631fed1679b
Author: Pavel Stehule 
Date:   Sun Mar 15 00:53:49 2015 +0100

initial

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index fdfb431..2a0d50f 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -134,7 +134,8 @@ static void expand_schema_name_patterns(Archive *fout,
 			SimpleOidList *oids);
 static void expand_table_name_patterns(Archive *fout,
 		   SimpleStringList *patterns,
-		   SimpleOidList *oids);
+		   SimpleOidList *oids,
+		   bool strict_table_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);
@@ -253,6 +254,8 @@ static char *get_synchronized_snapshot(Archive *fout);
 static PGresult *ExecuteSqlQueryForSingleRow(Archive *fout, char *query);
 static void setupDumpWorker(Archive *AHX, DumpOptions *dopt, RestoreOptions *ropt);
 
+static int			strict_table_names = false;
+
 
 int
 main(int argc, char **argv)
@@ -332,6 +335,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", no_argument, &strict_table_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},
@@ -700,15 +704,18 @@ main(int argc, char **argv)
 	if (table_include_patterns.head != NULL)
 	{
 		expand_table_name_patterns(fout, &table_include_patterns,
-   &table_include_oids);
+   &table_include_oids,
+   strict_table_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 */
 
@@ -1173,13 +1180,30 @@ expand_schema_name_patterns(Archive *fout,
 	destroyPQExpBuffer(query);
 }
 
+static

Re: [HACKERS] pg_dump quietly ignore missing tables - is it bug?

[Pavel Stehule]

2015-03-13 23:43 GMT+01:00 Josh Berkus :

> On 03/13/2015 10:01 AM, Pavel Stehule wrote:
> >
> >
> > 2015-03-13 17:39 GMT+01:00 Robert Haas  > >:
> >
> > On Fri, Mar 13, 2015 at 11:26 AM, Pavel Stehule
> > mailto:pavel.steh...@gmail.com>> wrote:
> > > we found possible bug in pg_dump. It raise a error only when all
> > specified
> > > tables doesn't exists. When it find any table, then ignore missing
> > other.
> > >
> > > /usr/local/pgsql/bin/pg_dump -t Foo -t omega -s postgres >
> > /dev/null; echo
> > > $?
> > >
> > > foo doesn't exists - it creates broken backup due missing "Foo"
> table
> > >
> > >  [pavel@localhost include]$ /usr/local/pgsql/bin/pg_dump -t Foo -t
> > omegaa -s
> > > postgres > /dev/null; echo $?
> > > pg_dump: No matching tables were found
> > > 1
> > >
> > > Is it ok? I am thinking, so it is potentially dangerous. Any
> > explicitly
> > > specified table should to exists.
> >
> > Keep in mind that the argument to -t is a pattern, not just a table
> > name.  I'm not sure how much that affects the calculus here, but it's
> > something to think about.
> >
> >
> > yes, it has a sense, although now, I am don't think so it was a good
> > idea. There should be some difference between table name and table
> pattern.
>
> There was a long discussion about this when the feature was introduced
> in 7.3(?) IIRC.  Changing it now would break backwards-compatibility, so
> you'd really need to introduce a new option.
>
> And, if you introduce a new option which is a specific table name, would
> you require a schema name or not?
>

We can use a same rules like we use for STRICT clause somewhere. There
should be only one table specified by the option. If it is not necessary,
then only name is enough, else qualified name is necessary.

what is a name for this option? Maybe only with long name - some like
'required-table' ?

Regards

Pavel


>
> --
> Josh Berkus
> PostgreSQL Experts Inc.
> http://pgexperts.com
>

Re: [HACKERS] pg_dump quietly ignore missing tables - is it bug?

[Josh Berkus]

On 03/13/2015 10:01 AM, Pavel Stehule wrote:
> 
> 
> 2015-03-13 17:39 GMT+01:00 Robert Haas  >:
> 
> On Fri, Mar 13, 2015 at 11:26 AM, Pavel Stehule
> mailto:pavel.steh...@gmail.com>> wrote:
> > we found possible bug in pg_dump. It raise a error only when all
> specified
> > tables doesn't exists. When it find any table, then ignore missing
> other.
> >
> > /usr/local/pgsql/bin/pg_dump -t Foo -t omega -s postgres >
> /dev/null; echo
> > $?
> >
> > foo doesn't exists - it creates broken backup due missing "Foo" table
> >
> >  [pavel@localhost include]$ /usr/local/pgsql/bin/pg_dump -t Foo -t
> omegaa -s
> > postgres > /dev/null; echo $?
> > pg_dump: No matching tables were found
> > 1
> >
> > Is it ok? I am thinking, so it is potentially dangerous. Any
> explicitly
> > specified table should to exists.
> 
> Keep in mind that the argument to -t is a pattern, not just a table
> name.  I'm not sure how much that affects the calculus here, but it's
> something to think about.
> 
> 
> yes, it has a sense, although now, I am don't think so it was a good
> idea. There should be some difference between table name and table pattern.

There was a long discussion about this when the feature was introduced
in 7.3(?) IIRC.  Changing it now would break backwards-compatibility, so
you'd really need to introduce a new option.

And, if you introduce a new option which is a specific table name, would
you require a schema name or not?

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_dump quietly ignore missing tables - is it bug?

[David G. Johnston]

On Fri, Mar 13, 2015 at 10:01 AM, Pavel Stehule 
wrote:

>
>
> 2015-03-13 17:39 GMT+01:00 Robert Haas :
>
>> On Fri, Mar 13, 2015 at 11:26 AM, Pavel Stehule 
>> wrote:
>> > we found possible bug in pg_dump. It raise a error only when all
>> specified
>> > tables doesn't exists. When it find any table, then ignore missing
>> other.
>> >
>> > /usr/local/pgsql/bin/pg_dump -t Foo -t omega -s postgres > /dev/null;
>> echo
>> > $?
>> >
>> > foo doesn't exists - it creates broken backup due missing "Foo" table
>> >
>> >  [pavel@localhost include]$ /usr/local/pgsql/bin/pg_dump -t Foo -t
>> omegaa -s
>> > postgres > /dev/null; echo $?
>> > pg_dump: No matching tables were found
>> > 1
>> >
>> > Is it ok? I am thinking, so it is potentially dangerous. Any explicitly
>> > specified table should to exists.
>>
>> Keep in mind that the argument to -t is a pattern, not just a table
>> name.  I'm not sure how much that affects the calculus here, but it's
>> something to think about.
>>
>
> yes, it has a sense, although now, I am don't think so it was a good idea.
> There should be some difference between table name and table pattern.
>
>
​There is...a single table name is simply expressed as a pattern without
any wildcards.  The issue here is that pg_dump doesn't require that every
instance of -t find one (or more, if a wildcard is present) entries only
that at least one entry is found among all of the patterns specified by -t​.

I'll voice my agreement that each of the -t specifications should find at
least one table in order for the dump as a whole to succeed; though
depending on presented use cases for the current behavior I could see
allowing the command writer to specify a more lenient interpretation by
specifying something like --allow-missing-tables.

Command line switch formats don't really allow you to write "-t?​" to mean
"I want these table(s) if present", do they?  I guess the input itself
could be interpreted that way though; a leading "?" is not a valid wildcard
and double-quotes would be required for it to be a valid table name.

David J.

Re: [HACKERS] pg_dump quietly ignore missing tables - is it bug?

[Pavel Stehule]

2015-03-13 17:39 GMT+01:00 Robert Haas :

> On Fri, Mar 13, 2015 at 11:26 AM, Pavel Stehule 
> wrote:
> > we found possible bug in pg_dump. It raise a error only when all
> specified
> > tables doesn't exists. When it find any table, then ignore missing other.
> >
> > /usr/local/pgsql/bin/pg_dump -t Foo -t omega -s postgres > /dev/null;
> echo
> > $?
> >
> > foo doesn't exists - it creates broken backup due missing "Foo" table
> >
> >  [pavel@localhost include]$ /usr/local/pgsql/bin/pg_dump -t Foo -t
> omegaa -s
> > postgres > /dev/null; echo $?
> > pg_dump: No matching tables were found
> > 1
> >
> > Is it ok? I am thinking, so it is potentially dangerous. Any explicitly
> > specified table should to exists.
>
> Keep in mind that the argument to -t is a pattern, not just a table
> name.  I'm not sure how much that affects the calculus here, but it's
> something to think about.
>

yes, it has a sense, although now, I am don't think so it was a good idea.
There should be some difference between table name and table pattern.

Regards

Pavel


>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Re: [HACKERS] pg_dump quietly ignore missing tables - is it bug?

[Robert Haas]

On Fri, Mar 13, 2015 at 11:26 AM, Pavel Stehule  wrote:
> we found possible bug in pg_dump. It raise a error only when all specified
> tables doesn't exists. When it find any table, then ignore missing other.
>
> /usr/local/pgsql/bin/pg_dump -t Foo -t omega -s postgres > /dev/null; echo
> $?
>
> foo doesn't exists - it creates broken backup due missing "Foo" table
>
>  [pavel@localhost include]$ /usr/local/pgsql/bin/pg_dump -t Foo -t omegaa -s
> postgres > /dev/null; echo $?
> pg_dump: No matching tables were found
> 1
>
> Is it ok? I am thinking, so it is potentially dangerous. Any explicitly
> specified table should to exists.

Keep in mind that the argument to -t is a pattern, not just a table
name.  I'm not sure how much that affects the calculus here, but it's
something to think about.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers