pg_dump quietly ignore missing tables - is it bug?

Started by Pavel Stehuleabout 11 years ago34 messageshackers
Jump to latest
#1Pavel Stehule
pavel.stehule@gmail.com

Hi

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.

Regards

Pavel

#2Robert Haas
robertmhaas@gmail.com
In reply to: Pavel Stehule (#1)
Re: pg_dump quietly ignore missing tables - is it bug?

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

#3Pavel Stehule
pavel.stehule@gmail.com
In reply to: Robert Haas (#2)
Re: pg_dump quietly ignore missing tables - is it bug?

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

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

Show quoted text

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

#4David G. Johnston
david.g.johnston@gmail.com
In reply to: Pavel Stehule (#3)
Re: pg_dump quietly ignore missing tables - is it bug?

On Fri, Mar 13, 2015 at 10:01 AM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

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

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

#5Josh Berkus
josh@agliodbs.com
In reply to: Pavel Stehule (#1)
Re: pg_dump quietly ignore missing tables - is it bug?

On 03/13/2015 10:01 AM, Pavel Stehule wrote:

2015-03-13 17:39 GMT+01:00 Robert Haas <robertmhaas@gmail.com
<mailto:robertmhaas@gmail.com>>:

On Fri, Mar 13, 2015 at 11:26 AM, Pavel Stehule
<pavel.stehule@gmail.com <mailto:pavel.stehule@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

#6Pavel Stehule
pavel.stehule@gmail.com
In reply to: Josh Berkus (#5)
Re: pg_dump quietly ignore missing tables - is it bug?

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

On 03/13/2015 10:01 AM, Pavel Stehule wrote:

2015-03-13 17:39 GMT+01:00 Robert Haas <robertmhaas@gmail.com
<mailto:robertmhaas@gmail.com>>:

On Fri, Mar 13, 2015 at 11:26 AM, Pavel Stehule
<pavel.stehule@gmail.com <mailto:pavel.stehule@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

Show quoted text

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

#7Pavel Stehule
pavel.stehule@gmail.com
In reply to: Pavel Stehule (#6)
Re: pg_dump quietly ignore missing tables - is it bug?

2015-03-14 19:33 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com>:

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

On 03/13/2015 10:01 AM, Pavel Stehule wrote:

2015-03-13 17:39 GMT+01:00 Robert Haas <robertmhaas@gmail.com
<mailto:robertmhaas@gmail.com>>:

On Fri, Mar 13, 2015 at 11:26 AM, Pavel Stehule
<pavel.stehule@gmail.com <mailto:pavel.stehule@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

Show quoted text

Regards

Pavel

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

Attachments:

pg_dump.patchtext/x-patch; charset=US-ASCII; name=pg_dump.patchDownload+61-25
#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Pavel Stehule (#7)
Re: pg_dump quietly ignore missing tables - is it bug?

Pavel Stehule <pavel.stehule@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

#9Pavel Stehule
pavel.stehule@gmail.com
In reply to: Tom Lane (#8)
Re: pg_dump quietly ignore missing tables - is it bug?

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

Pavel Stehule <pavel.stehule@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

Show quoted text

regards, tom lane

#10Pavel Stehule
pavel.stehule@gmail.com
In reply to: Tom Lane (#8)
Re: pg_dump quietly ignore missing tables - is it bug?

Hi

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

Pavel Stehule <pavel.stehule@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

Show quoted text

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

Attachments:

pg_dump-stricter.patchtext/x-patch; charset=US-ASCII; name=pg_dump-stricter.patchDownload+93-25
#11Pavel Stehule
pavel.stehule@gmail.com
In reply to: Pavel Stehule (#10)
Re: pg_dump quietly ignore missing tables - is it bug?

2015-03-23 17:11 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com>:

Hi

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

Pavel Stehule <pavel.stehule@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

Show quoted text

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

Attachments:

pg_dump-strict-3.patchtext/x-patch; charset=US-ASCII; name=pg_dump-strict-3.patchDownload+198-151
#12Shulgin, Oleksandr
oleksandr.shulgin@zalando.de
In reply to: Pavel Stehule (#11)
Re: pg_dump quietly ignore missing tables - is it bug?

Pavel Stehule <pavel.stehule@gmail.com> writes:

2015-03-23 17:11 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com>:

Hi

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

Pavel Stehule <pavel.stehule@gmail.com> writes:

other variant, I hope better than previous. We can introduce new long
option "--strict". With this active option, every pattern specified by

-t

option have to have identifies exactly only one table. It can be used

for

any other "should to exists" patterns - schemas. Initial implementation

in

attachment.

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

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

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

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

attached initial implementation

updated version - same mechanism should be used for schema

Hello,

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

Also, the new --table-if-exists options seems to be doing what the old
--table did, and I'm not really sure I underestand what --table does
now.

I propose instead to add a separate new option --strict-include, without
argument, that only controls the behavior when an include pattern didn't
find any table (or schema).

Please see attached patch.

--
Alex

Attachments:

pg_dump-strict-include-4.patchtext/x-diffDownload+105-69
#13Pavel Stehule
pavel.stehule@gmail.com
In reply to: Shulgin, Oleksandr (#12)
Re: pg_dump quietly ignore missing tables - is it bug?

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

Pavel Stehule <pavel.stehule@gmail.com> writes:

2015-03-23 17:11 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com>:

Hi

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

Pavel Stehule <pavel.stehule@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

Show quoted text

Please see attached patch.

--
Alex

#14Shulgin, Oleksandr
oleksandr.shulgin@zalando.de
In reply to: Pavel Stehule (#13)
Re: pg_dump quietly ignore missing tables - is it bug?

On Fri, May 22, 2015 at 6:09 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

2015-05-21 16:48 GMT+02:00 Oleksandr Shulgin <oleksandr.shulgin@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

#15Pavel Stehule
pavel.stehule@gmail.com
In reply to: Shulgin, Oleksandr (#14)
Re: pg_dump quietly ignore missing tables - is it bug?

2015-05-22 18:30 GMT+02:00 Shulgin, Oleksandr <oleksandr.shulgin@zalando.de>
:

On Fri, May 22, 2015 at 6:09 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

2015-05-21 16:48 GMT+02:00 Oleksandr Shulgin <
oleksandr.shulgin@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

Show quoted text

--
Alex

#16Tom Lane
tgl@sss.pgh.pa.us
In reply to: Shulgin, Oleksandr (#12)
Re: pg_dump quietly ignore missing tables - is it bug?

Oleksandr Shulgin <oleksandr.shulgin@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

#17Shulgin, Oleksandr
oleksandr.shulgin@zalando.de
In reply to: Pavel Stehule (#15)
Re: pg_dump quietly ignore missing tables - is it bug?

On Fri, May 22, 2015 at 6:32 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

2015-05-22 18:30 GMT+02:00 Shulgin, Oleksandr <
oleksandr.shulgin@zalando.de>:

On Fri, May 22, 2015 at 6:09 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

2015-05-21 16:48 GMT+02:00 Oleksandr Shulgin <
oleksandr.shulgin@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.

#18Pavel Stehule
pavel.stehule@gmail.com
In reply to: Shulgin, Oleksandr (#17)
Re: pg_dump quietly ignore missing tables - is it bug?

2015-05-22 18:35 GMT+02:00 Shulgin, Oleksandr <oleksandr.shulgin@zalando.de>
:

On Fri, May 22, 2015 at 6:32 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

2015-05-22 18:30 GMT+02:00 Shulgin, Oleksandr <
oleksandr.shulgin@zalando.de>:

On Fri, May 22, 2015 at 6:09 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

2015-05-21 16:48 GMT+02:00 Oleksandr Shulgin <
oleksandr.shulgin@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

#19Pavel Stehule
pavel.stehule@gmail.com
In reply to: Tom Lane (#16)
Re: pg_dump quietly ignore missing tables - is it bug?

2015-05-22 18:34 GMT+02:00 Tom Lane <tgl@sss.pgh.pa.us>:

Oleksandr Shulgin <oleksandr.shulgin@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

Show quoted text

regards, tom lane

#20Shulgin, Oleksandr
oleksandr.shulgin@zalando.de
In reply to: Tom Lane (#16)
Re: pg_dump quietly ignore missing tables - is it bug?

On Fri, May 22, 2015 at 6:34 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Oleksandr Shulgin <oleksandr.shulgin@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

#21Fujii Masao
masao.fujii@gmail.com
In reply to: Pavel Stehule (#19)
#22Pavel Stehule
pavel.stehule@gmail.com
In reply to: Fujii Masao (#21)
#23Pavel Stehule
pavel.stehule@gmail.com
In reply to: Pavel Stehule (#22)
#24Pavel Stehule
pavel.stehule@gmail.com
In reply to: Pavel Stehule (#23)
#25Pavel Stehule
pavel.stehule@gmail.com
In reply to: Pavel Stehule (#24)
#26Kyotaro Horiguchi
horikyota.ntt@gmail.com
In reply to: Pavel Stehule (#25)
#27Kyotaro Horiguchi
horikyota.ntt@gmail.com
In reply to: Kyotaro Horiguchi (#26)
#28Pavel Stehule
pavel.stehule@gmail.com
In reply to: Kyotaro Horiguchi (#26)
#29Heikki Linnakangas
heikki.linnakangas@enterprisedb.com
In reply to: Pavel Stehule (#28)
#30Pavel Stehule
pavel.stehule@gmail.com
In reply to: Heikki Linnakangas (#29)
#31Pavel Stehule
pavel.stehule@gmail.com
In reply to: Pavel Stehule (#30)
#32Jim Nasby
Jim.Nasby@BlueTreble.com
In reply to: Pavel Stehule (#31)
#33Pavel Stehule
pavel.stehule@gmail.com
In reply to: Jim Nasby (#32)
#34Michael Paquier
michael@paquier.xyz
In reply to: Pavel Stehule (#33)