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

2015-08-26 Thread Michael Paquier
On Sun, Aug 23, 2015 at 10:47 PM, Pavel Stehule pavel.steh...@gmail.com 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?

2015-08-23 Thread Pavel Stehule
Hi

2015-08-22 0:09 GMT+02:00 Jim Nasby jim.na...@bluetreble.com:

 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.

 +para
 + Require that table and/or schema match at least one entity each.
 + Without any entity in the database to be dumped, an error message
 + is printed and dump is aborted.
 +/para

 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.

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

 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
  /varlistentry
 
  varlistentry
+  termoption--strict-names//term
+  listitem
+   para
+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.
+   /para
+   para
+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.
+   /para
+  /listitem
+ /varlistentry
+
+ varlistentry
   termoption-T replaceable class=parametertable/replaceable/option/term
   termoption--exclude-table=replaceable class=parametertable/replaceable/option/term
   listitem
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 @@
  /varlistentry
 
  varlistentry
+  termoption--strict-names//term
+  listitem
+   para
+Require that each schema (-n / --schema) and table (-t / --table)
+qualifier match at least one schema/table in the backup file.
+   /para
+  /listitem
+ /varlistentry
+
+ varlistentry
   termoption-T replaceable class=parametertrigger/replaceable/option/term
   termoption--trigger=replaceable class=parametertrigger/replaceable/option/term
   listitem
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 

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

2015-08-13 Thread Pavel Stehule
Hi

2015-07-30 12:44 GMT+02:00 Heikki Linnakangas hlinn...@iki.fi:

 On 07/25/2015 07:08 PM, Pavel Stehule wrote:

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


 The documentation says the option is called --strict-names, while the code
 has --strict-mode. I like --strict-names more, mode seems redundant, and
 it's not clear what it's strict about.


ok


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


I'll do it


 Can we do better than issuing a separate query for each table/schema name?
 The performance of this isn't very important, but still it seems like you
 could fairly easily refactor the code to avoid that. Perhaps return an
 extra constant for part of the UNION to distinguish which result row came
 from which pattern, and check that at least one row is returned for each.


I did few tests and for 1K tables the union is faster about 50ms, but the
code is much more complex, for 10K tables, the union is significantly
slower (probably due planning) 2sec x 7sec. So if we are expecting backup
on not too slow network, then simple solution is winner - Postgres process
simple read queries quickly.

Regards

Pavel



 - Heikki




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

2015-08-13 Thread 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 pavel.steh...@gmail.com:

 Hi

 2015-07-30 12:44 GMT+02:00 Heikki Linnakangas hlinn...@iki.fi:

 On 07/25/2015 07:08 PM, Pavel Stehule wrote:

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


 The documentation says the option is called --strict-names, while the
 code has --strict-mode. I like --strict-names more, mode seems redundant,
 and it's not clear what it's strict about.


 ok


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


 I'll do it


 Can we do better than issuing a separate query for each table/schema
 name? The performance of this isn't very important, but still it seems like
 you could fairly easily refactor the code to avoid that. Perhaps return an
 extra constant for part of the UNION to distinguish which result row came
 from which pattern, and check that at least one row is returned for each.


 I did few tests and for 1K tables the union is faster about 50ms, but the
 code is much more complex, for 10K tables, the union is significantly
 slower (probably due planning) 2sec x 7sec. So if we are expecting backup
 on not too slow network, then simple solution is winner - Postgres process
 simple read queries quickly.

 Regards

 Pavel



 - Heikki



diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
new file mode 100644
index 7467e86..7c071fb
*** a/doc/src/sgml/ref/pg_dump.sgml
--- b/doc/src/sgml/ref/pg_dump.sgml
*** PostgreSQL documentation
*** 545,550 
--- 545,566 
   /varlistentry
  
   varlistentry
+   termoption--strict-names//term
+   listitem
+para
+ Require that table and/or schema match at least one entity each.
+ Without any entity in the database to be dumped, an error message
+ is printed and dump is aborted.
+/para
+para
+ This option has no effect on the exclude table and schema patterns
+ (and also option--exclude-table-data/): not matching any entities
+ isn't considered an error.
+/para
+   /listitem
+  /varlistentry
+ 
+  varlistentry
termoption-T replaceable class=parametertable/replaceable/option/term
termoption--exclude-table=replaceable class=parametertable/replaceable/option/term
listitem
diff --git a/doc/src/sgml/ref/pg_restore.sgml b/doc/src/sgml/ref/pg_restore.sgml
new file mode 100644
index 97e3420..9df7f69
*** a/doc/src/sgml/ref/pg_restore.sgml
--- b/doc/src/sgml/ref/pg_restore.sgml
***
*** 432,437 
--- 432,448 
   /varlistentry
  
   varlistentry
+   termoption--strict-names//term
+   listitem
+para
+ Require that table and/or schema and/or function and/or trigger match
+ at least one entity each. Without any entity in the backup file, 
+ an error message is printed and restore is aborted.
+/para
+   /listitem
+  /varlistentry
+ 
+  varlistentry
termoption-T replaceable class=parametertrigger/replaceable/option/term
termoption--trigger=replaceable class=parametertrigger/replaceable/option/term
listitem
diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
new file mode 100644
index d7506e1..52b2b98
*** a/src/bin/pg_dump/dumputils.c
--- b/src/bin/pg_dump/dumputils.c
*** simple_string_list_append(SimpleStringLi
*** 1220,1225 
--- 1220,1226 
  		pg_malloc(offsetof(SimpleStringListCell, val) +strlen(val) + 1);
  
  	cell-next = NULL;
+ 	cell-touched = false;
  	strcpy(cell-val, val);
  
  	if (list-tail)
*** simple_string_list_member(SimpleStringLi
*** 1237,1243 
--- 1238,1260 
  	for (cell = list-head; cell; cell = cell-next)
  	{
  		if (strcmp(cell-val, val) == 0)
+ 		{
+ 			cell-touched = true;
  			return true;
+ 		}
  	}
  	return false;
  }
+ 
+ const char *
+ simple_string_list_not_touched(SimpleStringList *list)
+ {
+ 	SimpleStringListCell *cell;
+ 
+ 	for (cell = list-head; cell; cell = cell-next)
+ 	{
+ 		if (!cell-touched)
+ 			return cell-val;
+ 	}
+ 	return NULL;
+ }
diff --git a/src/bin/pg_dump/dumputils.h b/src/bin/pg_dump/dumputils.h
new file mode 100644
index b176746..9f31bbc
*** a/src/bin/pg_dump/dumputils.h
--- b/src/bin/pg_dump/dumputils.h
*** typedef struct SimpleOidList
*** 38,43 
--- 38,45 
  typedef struct SimpleStringListCell
  {
  	struct SimpleStringListCell *next;
+ 	bool		touched;/* true, when this string was searched
+   and touched */
  	char		val[FLEXIBLE_ARRAY_MEMBER];		/* null-terminated string here */
  } SimpleStringListCell;
  
*** extern void set_dump_section(const char
*** 103,107 
--- 105,111 
  
  extern void 

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

2015-07-30 Thread 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?

2015-07-25 Thread 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 horiguchi.kyot...@lab.ntt.co.jp
:

 Hello,

   2015-07-19 20:54 GMT+02:00 Pavel Stehule pavel.steh...@gmail.com:
   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 
   /varlistentry
  
   varlistentry
+   termoption--strict-names//term
+   listitem
+para
+ Require that table and/or schema match at least one entity each.
+ Without any entity in the database to be dumped, an error message
+ is printed and dump is aborted.
+/para
+para
+ This option has no effect on the exclude table and schema patterns
+ (and also option--exclude-table-data/): not matching any entities
+ isn't considered an error.
+/para
+   /listitem
+  /varlistentry
+ 
+  varlistentry
termoption-T replaceable class=parametertable/replaceable/option/term
termoption--exclude-table=replaceable class=parametertable/replaceable/option/term
listitem
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);
  

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

2015-07-22 Thread Kyotaro HORIGUCHI
Hello,

  2015-07-19 20:54 GMT+02:00 Pavel Stehule pavel.steh...@gmail.com:
  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?

2015-07-22 Thread Kyotaro HORIGUCHI
Sorry for the bogus on bogus.

At Thu, 23 Jul 2015 14:22:59 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI 
horiguchi.kyot...@lab.ntt.co.jp wrote in 
20150723.142259.200902861.horiguchi.kyot...@lab.ntt.co.jp
 Hello,
 
   2015-07-19 20:54 GMT+02:00 Pavel Stehule pavel.steh...@gmail.com:
   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?

2015-07-19 Thread Pavel Stehule
2015-07-19 20:54 GMT+02:00 Pavel Stehule pavel.steh...@gmail.com:

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






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

2015-07-19 Thread Pavel Stehule
2015-07-19 21:08 GMT+02:00 Pavel Stehule pavel.steh...@gmail.com:



 2015-07-19 20:54 GMT+02:00 Pavel Stehule pavel.steh...@gmail.com:

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







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

2015-07-19 Thread 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 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
+  termoption--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
   termoption-t replaceable class=parametertable/replaceable/option/term
   termoption--table=replaceable class=parametertable/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 

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

2015-07-07 Thread Fujii Masao
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.

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?

2015-05-22 Thread Shulgin, Oleksandr
On Fri, May 22, 2015 at 6:09 PM, Pavel Stehule pavel.steh...@gmail.com
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.

--
Alex


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

2015-05-22 Thread Pavel Stehule
2015-05-22 18:35 GMT+02:00 Shulgin, Oleksandr oleksandr.shul...@zalando.de
:

 On Fri, May 22, 2015 at 6:32 PM, Pavel Stehule pavel.steh...@gmail.com
 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 pavel.steh...@gmail.com
 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?

2015-05-22 Thread Pavel Stehule
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 pavel.steh...@gmail.com
 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?

2015-05-22 Thread Shulgin, Oleksandr
On Fri, May 22, 2015 at 6:34 PM, Tom Lane t...@sss.pgh.pa.us wrote:

 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.


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?

2015-05-22 Thread Tom Lane
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.)

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?

2015-05-22 Thread Pavel Stehule
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

Pavel



 regards, tom lane



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

2015-05-22 Thread Pavel Stehule
2015-05-21 16:48 GMT+02:00 Oleksandr Shulgin oleksandr.shul...@zalando.de:

 Pavel Stehule pavel.steh...@gmail.com writes:
 
  2015-03-23 17:11 GMT+01:00 Pavel Stehule pavel.steh...@gmail.com:
 
  Hi
 
  2015-03-15 16:09 GMT+01:00 Tom Lane t...@sss.pgh.pa.us:
 
  Pavel Stehule pavel.steh...@gmail.com writes:
   other variant, I hope better than previous. We can introduce new long
   option --strict. With this active option, every pattern specified
 by
  -t
   option have to have identifies exactly only one table. It can be used
  for
   any other should to exists patterns - schemas. Initial
 implementation
  in
   attachment.
 
  I think this design is seriously broken.  If I have '-t foo*' the code
  should not prevent that from matching multiple tables.  What would the
 use
  case for such a restriction be?
 
  What would make sense to me is one or both of these ideas:
 
  * require a match for a wildcard-free -t switch
 
  * require at least one (not exactly one) match for a wildcarded -t
switch.
 
 
 
  attached initial implementation
 
 
  updated version - same mechanism should be used for schema

 Hello,

 I think this is a bit over-engineered (apart from the fact that
 processSQLNamePattern is also used in two dozen of places in
 psql/describe.c and all of them must be touched for this patch to
 compile).


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?

2015-05-22 Thread Shulgin, Oleksandr
On Fri, May 22, 2015 at 6:32 PM, Pavel Stehule pavel.steh...@gmail.com
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 pavel.steh...@gmail.com
 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?

2015-03-24 Thread Pavel Stehule
2015-03-23 17:11 GMT+01:00 Pavel Stehule pavel.steh...@gmail.com:

 Hi

 2015-03-15 16:09 GMT+01:00 Tom Lane t...@sss.pgh.pa.us:

 Pavel Stehule pavel.steh...@gmail.com writes:
  other variant, I hope better than previous. We can introduce new long
  option --strict. With this active option, every pattern specified by
 -t
  option have to have identifies exactly only one table. It can be used
 for
  any other should to exists patterns - schemas. Initial implementation
 in
  attachment.

 I think this design is seriously broken.  If I have '-t foo*' the code
 should not prevent that from matching multiple tables.  What would the use
 case for such a restriction be?

 What would make sense to me is one or both of these ideas:

 * require a match for a wildcard-free -t switch

 * require at least one (not exactly one) match for a wildcarded -t
   switch.



 attached initial implementation


updated version - same mechanism should be used for schema

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 SimpleStringList tabledata_exclude_patterns = {NULL, 

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

2015-03-23 Thread Pavel Stehule
Hi

2015-03-15 16:09 GMT+01:00 Tom Lane t...@sss.pgh.pa.us:

 Pavel Stehule pavel.steh...@gmail.com writes:
  other variant, I hope better than previous. We can introduce new long
  option --strict. With this active option, every pattern specified by -t
  option have to have identifies exactly only one table. It can be used for
  any other should to exists patterns - schemas. Initial implementation
 in
  attachment.

 I think this design is seriously broken.  If I have '-t foo*' the code
 should not prevent that from matching multiple tables.  What would the use
 case for such a restriction be?

 What would make sense to me is one or both of these ideas:

 * require a match for a wildcard-free -t switch

 * require at least one (not exactly one) match for a wildcarded -t
   switch.



attached initial implementation

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 pavel.steh...@gooddata.com
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, 

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

2015-03-19 Thread Pavel Stehule
2015-03-15 16:09 GMT+01:00 Tom Lane t...@sss.pgh.pa.us:

 Pavel Stehule pavel.steh...@gmail.com writes:
  other variant, I hope better than previous. We can introduce new long
  option --strict. With this active option, every pattern specified by -t
  option have to have identifies exactly only one table. It can be used for
  any other should to exists patterns - schemas. Initial implementation
 in
  attachment.

 I think this design is seriously broken.  If I have '-t foo*' the code
 should not prevent that from matching multiple tables.  What would the use
 case for such a restriction be?


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?

2015-03-15 Thread Tom Lane
Pavel Stehule pavel.steh...@gmail.com writes:
 other variant, I hope better than previous. We can introduce new long
 option --strict. With this active option, every pattern specified by -t
 option have to have identifies exactly only one table. It can be used for
 any other should to exists patterns - schemas. Initial implementation in
 attachment.

I think this design is seriously broken.  If I have '-t foo*' the code
should not prevent that from matching multiple tables.  What would the use
case for such a restriction be?

What would make sense to me is one or both of these ideas:

* require a match for a wildcard-free -t switch

* require at least one (not exactly one) match for a wildcarded -t
  switch.

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?

2015-03-14 Thread Pavel Stehule
2015-03-14 19:33 GMT+01:00 Pavel Stehule pavel.steh...@gmail.com:



 2015-03-13 23:43 GMT+01:00 Josh Berkus j...@agliodbs.com:

 On 03/13/2015 10:01 AM, Pavel Stehule wrote:
 
 
  2015-03-13 17:39 GMT+01:00 Robert Haas robertmh...@gmail.com
  mailto:robertmh...@gmail.com:
 
  On Fri, Mar 13, 2015 at 11:26 AM, Pavel Stehule
  pavel.steh...@gmail.com 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 pavel.steh...@gooddata.com
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 void
+append_table_name_query(Archive *fout, PQExpBuffer query, char *tablename)
+{
+	

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

2015-03-14 Thread Pavel Stehule
2015-03-13 23:43 GMT+01:00 Josh Berkus j...@agliodbs.com:

 On 03/13/2015 10:01 AM, Pavel Stehule wrote:
 
 
  2015-03-13 17:39 GMT+01:00 Robert Haas robertmh...@gmail.com
  mailto:robertmh...@gmail.com:
 
  On Fri, Mar 13, 2015 at 11:26 AM, Pavel Stehule
  pavel.steh...@gmail.com 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?

2015-03-13 Thread David G. Johnston
On Fri, Mar 13, 2015 at 10:01 AM, Pavel Stehule pavel.steh...@gmail.com
wrote:



 2015-03-13 17:39 GMT+01:00 Robert Haas robertmh...@gmail.com:

 On Fri, Mar 13, 2015 at 11:26 AM, Pavel Stehule 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 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?

2015-03-13 Thread Josh Berkus
On 03/13/2015 10:01 AM, Pavel Stehule wrote:
 
 
 2015-03-13 17:39 GMT+01:00 Robert Haas robertmh...@gmail.com
 mailto:robertmh...@gmail.com:
 
 On Fri, Mar 13, 2015 at 11:26 AM, Pavel Stehule
 pavel.steh...@gmail.com 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?

2015-03-13 Thread Robert Haas
On Fri, Mar 13, 2015 at 11:26 AM, Pavel Stehule 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.

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


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

2015-03-13 Thread Pavel Stehule
2015-03-13 17:39 GMT+01:00 Robert Haas robertmh...@gmail.com:

 On Fri, Mar 13, 2015 at 11:26 AM, Pavel Stehule 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.

Regards

Pavel



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