BUG #16492: DROP VIEW IF EXISTS error

On Friday, June 12, 2020, PG Bug reporting form <noreply@postgresql.org>
wrote:

The following bug has been logged on the website:

Bug reference: 16492
Logged by: Nina Marlow
Email address: postgresql.2020@t-net.ruhr
PostgreSQL version: 12.3
Operating system: Linux/Docker
Description:

When trying to use "DROP VIEW IF EXISTS x" while a table named "x" exists,
the DROP statement raises an "Error: x is not a view".

The documentation says about the "IF EXISTS" clause:

Do not throw an error if the view does not exist. A notice is issued in

this case.

Imho, there shouldn't be an error if the view does not exist regardless of
whether or not a table with this name exists. The observed behavior is
contrary to the sense of the "IF EXISTS" clause as one would have to check
whether a table with the desired name exists before using "DROP VIEW IF
EXISTS". But then you could directly check whether or not the view itself
exists.

This thread from 2018 illustrates the current state of the discussion:

/messages/by-id/CAFj8pRD-Lq=9ZTHuAdq1RYW4EN0O5Pr0cvCUZs5=MUzs2dLCXQ@mail.gmail.com

I still concur this is indeed a bug that should be fixed.

David J.

pá 12. 6. 2020 v 21:43 odesílatel David G. Johnston <
david.g.johnston@gmail.com> napsal:

On Friday, June 12, 2020, PG Bug reporting form <noreply@postgresql.org>
wrote:

The following bug has been logged on the website:

Bug reference: 16492
Logged by: Nina Marlow
Email address: postgresql.2020@t-net.ruhr
PostgreSQL version: 12.3
Operating system: Linux/Docker
Description:

When trying to use "DROP VIEW IF EXISTS x" while a table named "x" exists,
the DROP statement raises an "Error: x is not a view".

The documentation says about the "IF EXISTS" clause:

Do not throw an error if the view does not exist. A notice is issued in

this case.

Imho, there shouldn't be an error if the view does not exist regardless of
whether or not a table with this name exists. The observed behavior is
contrary to the sense of the "IF EXISTS" clause as one would have to check
whether a table with the desired name exists before using "DROP VIEW IF
EXISTS". But then you could directly check whether or not the view itself
exists.

This thread from 2018 illustrates the current state of the discussion:

/messages/by-id/CAFj8pRD-Lq=9ZTHuAdq1RYW4EN0O5Pr0cvCUZs5=MUzs2dLCXQ@mail.gmail.com

I still concur this is indeed a bug that should be fixed.

It is hard to find an agreement here. Both alternatives has benefits and
disadvantages. Just depends on personal view. One direction is more
tolerant behaviour (more user friendly). Second direction is more strict
behaviour (developer friendly).
Probably preferences depends if somebody use DROP as an isolated command or
not. I use DROP IF EXISTS together with CREATE statement, and for me is
natural current design.

DROP TABLE IF EXISTS xxx;
CREATE TABLE xxx;

If the first statement doesn't fail, then the second statement will be
successful with very high priority. For me is little bit more intuitive
message "cannot to drop some" then "cannot to create some" when first
command is DROP, and I have to investigate, why DROP was ignored.

Because I use these statements typically in pair, then I prefer current
behaviour. I am not sure what is the best design here, but I don't think so
this case is a bug. It is just ambiguous.

Nice evening

Pavel

On Friday, June 12, 2020, Pavel Stehule <pavel.stehule@gmail.com> wrote:

DROP TABLE IF EXISTS xxx;
CREATE TABLE xxx;

If the first statement doesn't fail, then the second statement will be
successful with very high priority. For me is little bit more intuitive
message "cannot to drop some" then "cannot to create some" when first
command is DROP, and I have to investigate, why DROP was ignored.

Fixing this bug you’d still get: Error: cannot create table xxx, view with
same name already exists. Do you seriously expect a user to then ask why
the drop table command didn’t tell them about the view with the same name?

The create command should deal with namespace sharing, the drop command
just should do what is written on the tin. Especially since that is all it
is documented to be concerned with. As demonstrated actual use cases are
broken with the current behavior which exists seemingly to only try and
reduce user confusion. I’d rather have the defined and expected behavior
here and deal with confused people on the mailing list then tell people on
their valid uses are not as important.

David J.

pá 12. 6. 2020 v 22:34 odesílatel David G. Johnston <
david.g.johnston@gmail.com> napsal:

On Friday, June 12, 2020, Pavel Stehule <pavel.stehule@gmail.com> wrote:

DROP TABLE IF EXISTS xxx;
CREATE TABLE xxx;

If the first statement doesn't fail, then the second statement will be
successful with very high priority. For me is little bit more intuitive
message "cannot to drop some" then "cannot to create some" when first
command is DROP, and I have to investigate, why DROP was ignored.

Fixing this bug you’d still get: Error: cannot create table xxx, view with
same name already exists. Do you seriously expect a user to then ask why
the drop table command didn’t tell them about the view with the same name?

The create command should deal with namespace sharing, the drop command
just should do what is written on the tin. Especially since that is all it
is documented to be concerned with. As demonstrated actual use cases are
broken with the current behavior which exists seemingly to only try and
reduce user confusion. I’d rather have the defined and expected behavior
here and deal with confused people on the mailing list then tell people on
their valid uses are not as important.

If we change the behaviour then other group of users will be confused in
other cases. For me - this case is ambiguous, and the change doesn't do
things better for all.

I try to think about it from a different perspective and I don't see any
result. Minimally

postgres=# create table xxx (a int);
CREATE TABLE
postgres=# drop view xxx;
ERROR: "xxx" is not a view
HINT: Use DROP TABLE to remove a table.

DROP TABLE IF EXISTS and DROP TABLE are consistent now. The message is
""xxx" is not a view", it is not ""xxx" doesn't exist".

I would like to see the opinions of other people. I don't see "fixing this
case" as a clean win. I see the benefits and disadvantages.

Pavel

On Fri, Jun 12, 2020 at 2:16 PM Pavel Stehule <pavel.stehule@gmail.com>
wrote:

pá 12. 6. 2020 v 22:34 odesílatel David G. Johnston <
david.g.johnston@gmail.com> napsal:

On Friday, June 12, 2020, Pavel Stehule <pavel.stehule@gmail.com> wrote:

DROP TABLE IF EXISTS xxx;
CREATE TABLE xxx;

If the first statement doesn't fail, then the second statement will be
successful with very high priority. For me is little bit more intuitive
message "cannot to drop some" then "cannot to create some" when first
command is DROP, and I have to investigate, why DROP was ignored.

Fixing this bug you’d still get: Error: cannot create table xxx, view
with same name already exists. Do you seriously expect a user to then ask
why the drop table command didn’t tell them about the view with the same
name?

The create command should deal with namespace sharing, the drop command
just should do what is written on the tin. Especially since that is all it
is documented to be concerned with. As demonstrated actual use cases are
broken with the current behavior which exists seemingly to only try and
reduce user confusion. I’d rather have the defined and expected behavior
here and deal with confused people on the mailing list then tell people on
their valid uses are not as important.

If we change the behaviour then other group of users will be confused in
other cases.

This isn't a matter of one group of users being confused over another. The
people reporting this as a bug have a valid behavioral complaint that
prevents a valid use case. The people we might be simply confusing are
just going to get an error in a different location - they will still have
to deal with their underlying naming issues caused by the shared relation
namespace.

Preventing confusion in the presence of a true namespace conflict is not a
superior objective compared to having this feature function in the manner
that is both explicitly documented and implied by its name. You will need
to describe a case where confusion leads to an actual problem if you wish
to convince me otherwise.

For me - this case is ambiguous, and the change doesn't do things better
for all.

Yes, for most people it isn't better, just different - the error moves from
the DROP IF EXISTS to the subsequent CREATE. But for the minority of
people that simply want to ensure that a VIEW, and only a VIEW, of that
name doesn't exist this is a win. Sure, they have other options, querying
the catalog in a DO block, but the IF EXISTS feature says its supposed to
work for this purpose and they rightly are telling us it is a bug.

I try to think about it from a different perspective and I don't see any

result. Minimally

postgres=# create table xxx (a int);
CREATE TABLE
postgres=# drop view xxx;
ERROR: "xxx" is not a view
HINT: Use DROP TABLE to remove a table.

DROP TABLE IF EXISTS and DROP TABLE are consistent now. The message is
""xxx" is not a view", it is not ""xxx" doesn't exist".

Why is that consistency a good thing? If the view "xxx" doesn't exist:

DROP VIEW IF EXISTS should only cause a failure if it actually attempts to
perform a drop and then during the dropping execution encounters a
problem. If it never attempts to perform a physical drop it succeeds and
the state of the database is just as the user expects, the VIEW "xxx" is
not present. The command has an implicit: "otherwise do nothing" - this is
a bug precisely because we don't do nothing, we also check for a namespace
conflict when that is immaterial to the operation at hand.

DROP VIEW "xxx" - says "you better drop this view, if you cannot for any
reason, including you cannot locate the view, abort". We could have gone
ahead and made DROP VIEW a no-op if it didn't find a target but we don't,
we have IF EXISTS for this.

I'll support not back-patching this and doing a back-patch only doc fix but
nothing I came up with or heard back then or now suggests to me that the
current behavior is superior.

David J.

DROP TABLE IF EXISTS and DROP TABLE are consistent now. The message is
""xxx" is not a view", it is not ""xxx" doesn't exist".

Yes, but according to the documentation, there shouldn't be an error at all ("Do not throw an error if the *view* does not exist."). It doesn't say, "do not throw an error if the *object* does not exist", but "if the *view* does not exist". This is very clear to me.

So it's either a documentation error or a code error. It cannot be none.

so 13. 6. 2020 v 13:41 odesílatel Nina Marlow <postgresql.2020@t-net.ruhr>
napsal:

DROP TABLE IF EXISTS and DROP TABLE are consistent now. The message is
""xxx" is not a view", it is not ""xxx" doesn't exist".

Yes, but according to the documentation, there shouldn't be an error at
all ("Do not throw an error if the *view* does not exist."). It doesn't
say, "do not throw an error if the *object* does not exist", but "if the
*view* does not exist". This is very clear to me.

So it's either a documentation error or a code error. It cannot be none.

I prefer to fix the documentation. Current behavior looks a little bit
more practical and a little bit more safe, although I can understand very
well the different opinion.

On Sunday, June 14, 2020, Pavel Stehule <pavel.stehule@gmail.com> wrote:

so 13. 6. 2020 v 13:41 odesílatel Nina Marlow <postgresql.2020@t-net.ruhr>
napsal:

DROP TABLE IF EXISTS and DROP TABLE are consistent now. The message is
""xxx" is not a view", it is not ""xxx" doesn't exist".

Yes, but according to the documentation, there shouldn't be an error at
all ("Do not throw an error if the *view* does not exist."). It doesn't
say, "do not throw an error if the *object* does not exist", but "if the
*view* does not exist". This is very clear to me.

So it's either a documentation error or a code error. It cannot be none.

I prefer to fix the documentation. Current behavior looks a little bit
more practical and a little bit more safe, although I can understand very
well the different opinion.

How is the proposed behavior more risky? And no, the current behavior does
not have any uniquely practical use. Its only benefit is that it is how
things have worked forever and that is only because its flaws are rarely
encountered in practice.

David J.

"David G. Johnston" <david.g.johnston@gmail.com> writes:

How is the proposed behavior more risky? And no, the current behavior does
not have any uniquely practical use. Its only benefit is that it is how
things have worked forever and that is only because its flaws are rarely
encountered in practice.

I'm a little skeptical about the proposed change being of any benefit.
The usual reason for doing DROP IF EXISTS is that you're about to replace
the object. It will not help for the DROP to succeed if the conflicting
object is still there, because the CREATE is going to fail anyway. Thus,
the most likely effect of such a change is that we fix no scripts, while
breaking any scripts that were dependent on the existing behavior.

What I'd prefer to see, I think, is a command DROP RELATION [IF EXISTS]
that is entirely un-picky about the object's relkind. Once upon a time
DROP TABLE worked that way, IIRC, but it was "improved" with little
thought about the needs of schema-update scripts.

regards, tom lane

On Sun, Jun 14, 2020 at 10:12 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

"David G. Johnston" <david.g.johnston@gmail.com> writes:

How is the proposed behavior more risky? And no, the current behavior

does

not have any uniquely practical use. Its only benefit is that it is how
things have worked forever and that is only because its flaws are rarely
encountered in practice.

I'm a little skeptical about the proposed change being of any benefit.

Apparently some people want to use it and I've yet to see any downside
regardless of how marginal that use case may be.

IF EXISTS exists for script simplicity - not having to know exactly what
was present previously in the database.

I have a TABLE. At some point in the future I want to "turn it" into a
VIEW. They should have the same name. I also need to be able to rebuild
the VIEW.

Initial:
TABLE exists; View does not
DROP TABLE IF EXISTS name; -- drops
DROP VIEW IF EXISTS name; -- no-op
-- in short, I know about the namespace problem and am happy ensuring that
the namespace does not contain the name I care about after these drop
commands run..
CREATE VIEW name; -- creates

Subsequent (this fails when it shouldn't):
TABLE does not exist; VIEW exists
DROP TABLE IF EXISTS name; -- error!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
DROP VIEW IF EXISTS name; -- never gets here - it should though
CREATE VIEW name; -- creates/rebuilds the view

Regardless of how marginal you believe the above to be I don't see any
reason why it cannot be supported.

People demonstrably want to do something like this, and because DROP
RELATION doesn't exist and the documentation says it should work they
rightly complain. The fix seems simple enough and without risk - even if
behavior changes a little bit. Other bug fix patches have contributed even
larger behavioral changes and have been back-patched, which I believe this
should be as well but I do accept the arguments against as at least
warranting only a fix of head.

The usual reason for doing DROP IF EXISTS is that you're about to replace
the object. It will not help for the DROP to succeed if the conflicting
object is still there, because the CREATE is going to fail anyway.

Right, so the fact that DROP provokes the same error in this case is not
useful.

Thus,

the most likely effect of such a change is that we fix no scripts, while
breaking any scripts that were dependent on the existing behavior.

We cause scripts that wouldn't work before to now work as the author wanted
in the first place. And we don't "break" any scripts - i.e., issue an
error where one wasn't being issued before.

Sure, I suppose someone could have written:

psql -c "DROP TABLE IF EXISTS name"
if [ $? != 0 ]; then

fi
psql -c "CREATE TABLE"
# the above should never fail so do not error handling here...

But I'm doubtful, and would consider catering to that crowd less than
desirable.

What I'd prefer to see, I think, is a command DROP RELATION [IF EXISTS]
that is entirely un-picky about the object's relkind. Once upon a time
DROP TABLE worked that way, IIRC, but it was "improved" with little
thought about the needs of schema-update scripts.

And this is where things stalled out last time. If you feel strongly
enough that this needs to stay this way until a superior and more invasive
patch is submitted can you please at least fix the documentation bug?

David J.

On Sunday, June 14, 2020, Tom Lane <tgl@sss.pgh.pa.us> wrote:

What I'd prefer to see, I think, is a command DROP RELATION [IF EXISTS]
that is entirely un-picky about the object's relkind. Once upon a time
DROP TABLE worked that way, IIRC, but it was "improved" with little
thought about the needs of schema-update scripts

IMO if the DROP type IF EXISTS worked as expected the drop relation command
would be undesirable - the use case being that there is a specific type of
object the user was potentially expecting to exist that they want to ensure
does not. If an object of a different typed gets matched for some reason
it should not get dropped as a safety feature.

Unless drop relation is part of the SQL standard I’d suggest it is
something to not implement on “don’t give users a loaded weapon” grounds.
People aren’t clamoring for it and making the drop type if exists work in
the proposed manner makes it possible to, if a bit verbosely, replicate the
same behavior in the rare case it would be needed. The same rarity that is
being used to defend not changing the behavior.

David J.

the most likely effect of such a change is that we fix no scripts, while
breaking any scripts that were dependent on the existing behavior.

As the documentation seems to always have said that "IF EXISTS" doesn't raise an error, there's no script that could get broken.

On the other side, currently I currently don't see a way of dropping a view or table without knowing its exact type.

So to drop a *view*, I need to be sure that there is no *table* with the same name. I have to check that first before using DROP. But that makes IF EXISTS more or less useless because I might just as well check whether the view exists and depending on the result either do a DROP or not.

What I'd prefer to see, I think, is a command DROP RELATION [IF EXISTS]

If the already implemented DROPs behaved as documented, you'd not need an additional third statement. It'd be just identical to

DROP TABLE IF EXISTS
DROP VIEW IF EXISTS

po 15. 6. 2020 v 8:51 odesílatel Nina Marlow <postgresql.2020@t-net.ruhr>
napsal:

the most likely effect of such a change is that we fix no scripts, while
breaking any scripts that were dependent on the existing behavior.

As the documentation seems to always have said that "IF EXISTS" doesn't
raise an error, there's no script that could get broken.

On the other side, currently I currently don't see a way of dropping a
view or table without knowing its exact type.

So to drop a *view*, I need to be sure that there is no *table* with the
same name. I have to check that first before using DROP. But that makes IF
EXISTS more or less useless because I might just as well check whether the
view exists and depending on the result either do a DROP or not.

It is harder when you introduce schemas and search_path.

We know so in one schema there cannot be view and table with same name, but
you can have more schemas on search_path

So the behaviour can be little bit different if you use qualified name or
not

On Monday, June 15, 2020, Pavel Stehule <pavel.stehule@gmail.com> wrote:

po 15. 6. 2020 v 8:51 odesílatel Nina Marlow <postgresql.2020@t-net.ruhr>
napsal:

the most likely effect of such a change is that we fix no scripts, while
breaking any scripts that were dependent on the existing behavior.

As the documentation seems to always have said that "IF EXISTS" doesn't
raise an error, there's no script that could get broken.

On the other side, currently I currently don't see a way of dropping a
view or table without knowing its exact type.

So to drop a *view*, I need to be sure that there is no *table* with the
same name. I have to check that first before using DROP. But that makes IF
EXISTS more or less useless because I might just as well check whether the
view exists and depending on the result either do a DROP or not.

It is harder when you introduce schemas and search_path.

We know so in one schema there cannot be view and table with same name,
but you can have more schemas on search_path

So the behaviour can be little bit different if you use qualified name or
not

Huh? The lack of concrete examples makes it difficult to take seriously
your defense of the current behavior.

David J.

po 15. 6. 2020 v 16:45 odesílatel David G. Johnston <
david.g.johnston@gmail.com> napsal:

On Monday, June 15, 2020, Pavel Stehule <pavel.stehule@gmail.com> wrote:

po 15. 6. 2020 v 8:51 odesílatel Nina Marlow <postgresql.2020@t-net.ruhr>
napsal:

the most likely effect of such a change is that we fix no scripts,

while

breaking any scripts that were dependent on the existing behavior.

As the documentation seems to always have said that "IF EXISTS" doesn't
raise an error, there's no script that could get broken.

On the other side, currently I currently don't see a way of dropping a
view or table without knowing its exact type.

So to drop a *view*, I need to be sure that there is no *table* with the
same name. I have to check that first before using DROP. But that makes IF
EXISTS more or less useless because I might just as well check whether the
view exists and depending on the result either do a DROP or not.

It is harder when you introduce schemas and search_path.

We know so in one schema there cannot be view and table with same name,
but you can have more schemas on search_path

So the behaviour can be little bit different if you use qualified name or
not

Huh? The lack of concrete examples makes it difficult to take seriously
your defense of the current behavior.

CREATE SCHEMA s1;
CREATE SCHEMA s2;

CREATE VIEW s1.rel AS SELECT 1;
CREATE TABLE s2.rel (a int, b int)

SET SEARCH_PATH TO s1, s2;

DROP TABLE IF EXISTS rel;
CREATE TABLE rel(a int, b int);

This is a synthetic example. But this case shows so the behaviour is same,
and search_path should not be calculated. This script fails in both cases
(both variants of DROP TABLE IF EXISTS) with same error messages like
example when search_path will not be used.

On Mon, Jun 15, 2020 at 8:06 AM Pavel Stehule <pavel.stehule@gmail.com>
wrote:

Huh? The lack of concrete examples makes it difficult to take seriously

your defense of the current behavior.

CREATE SCHEMA s1;
CREATE SCHEMA s2;

CREATE VIEW s1.rel AS SELECT 1;
CREATE TABLE s2.rel (a int, b int)

SET SEARCH_PATH TO s1, s2;

DROP TABLE IF EXISTS rel;
CREATE TABLE rel(a int, b int);

This is a synthetic example. But this case shows so the behaviour is same,
and search_path should not be calculated. This script fails in both cases
(both variants of DROP TABLE IF EXISTS) with same error messages like
example when search_path will not be used.

So the presence of a view of the name rel in the search_path s1 namespace
is matched against the drop table non-schema specific name rel and in both
cases the code stops looking and returns saying that it was unable to find
a table named rel?

**Given that it happens in both scenarios that wouldn't prevent making this
change. The create table still cannot execute successfully whether the if
exists actually errors out or does nothing.**

I cannot test the alternative so cannot confirm your "with same error
message" statement...but presently the drop table fails in your example
which is what I find to be more important. If the drop fails today the
create should fail today. That way if the drop stops failing the create
will continue to do so and the outcome of the script is the same. If the
drop doesn't fail the create might fail or it might not. I've already
stated that a change here presumes not caring about keeping the location
and message of the resulting failure the same, just whether a failure
occurs or not.

Your conclusion that "search_path will not be used" is wrong since that is
not possible - the only way to find anything in the system is via the
search_path. This is easily demonstrated since adding "DROP VIEW s1.rel;"
after the "SET search_path" results in the script working (kinda, see
below).

If you are saying that only the first search_path containing the named
relation in the namespace is considered that seems like debatable behavior
- and in any case is likewise neither undocumented nor intuitive. I would
indeed find that to be desirable as a safety feature and if you are saying
that the fixed if exists behavior would already work that way that is one
oh-by-the-way that I hadn't considered that "just works" - though it is
independent of the if exists behavior. So much so that I'd also say that
what you just showed is a problem in its own right - though arguably one of
the users own making for inconsistent usage of schema prefixes.

SET search_path to s1, s2
DROP VIEW IF EXISTS s1.rel; -- drops s1.rel so the if exists bug doesn't
come into play
DROP TABLE IF EXISTS rel; -- drops s2.rel, again dropping happens the same
in either case
CREATE TABLE rel; -- creates s1.rel

If the expectation is that the user is dropping and creating tables that
are supposed to have been logically the same (in the same namespace) the
fact that we dropped the table in s2 but created the new one is s1 is
undesirable.

Again, regardless of the desirability of the above it behaves identically
no matter how IF EXISTS behaves so it's not a reason to avoid fixing the
bug.

For the example above I would have expected the fixed IF EXISTS to be:
SET search_path to s1, s2;
DROP TABLE IF EXISTS rel; -- drops s2.rel
CREATE TABLE rel; -- fails to create table s1.rel do to namespace collision
but it still dropped s2.rel.

If that was/is the outcome then I'd say that indeed this is a concern
because now the namespace checks in s1 is preventing a valid drop from
happening in s2. But then we are back to the fact that the absence of a
namespace collision in s1 results in exactly that behavior. Though if all
of this is being done in a transaction then the dropping of s2.rel will
just be rolled back when s1.rel fails to be created. And apparently this
isn't the actual behavior, which itself is actually less broken in the face
of this change (i.e., I have no problem with an "ambiguous relation name"
error being raised here).

I'm now more solidly behind the idea of only doing this in v14 but this
doesn't really change my feelings on the fact that the existing behavior is
a bug. It prevents using a valid set of drop commands in series with the
only potentially problematic cases being those where the user is careless
with namespaces and doesn't do related things in a transaction. Improving
the logic and introducing the concept of "ambiguous relation name" would
increase the complexity of the fix but would be more user-friendly and
hopefully acceptable by the hacker community.

Maybe a good first step is to agree on what a documentation and possible
code comment would look like describing the current behavior since we are
going to need that for the current releases regardless.

David J.