Обсуждение: Unexpected behavior of DROP VIEW/TABLE IF EXISTS
Hi,
I want to delete a table X, that may not exist, hence I execute
DROP TABLE IF EXISTS X;
However, if X is a view, I get an error
ERROR: "X" is not a table
HINT: Use DROP VIEW to remove a view.
SQL state: 42809
That is unexpected and also difficult to handle, if I want to be sure
that I can delete all old tables *and* views called X, and create a new
view for instance with
CREATE VIEW X AS ....
I cannot do that safely, because having both DROP commands would for
sure cause an error and therefore a rollback.
What do you think, is it worth to create a patch to solve this issue,
where a DROP TABLE does not fail, if the given name is actually a VIEW
or vice-versa?
Best regards,
Peter
2018-06-26 17:23 GMT+02:00 Peter Moser <pitiz29a@gmail.com>:
Hi,
I want to delete a table X, that may not exist, hence I execute
DROP TABLE IF EXISTS X;
However, if X is a view, I get an error
ERROR: "X" is not a table
HINT: Use DROP VIEW to remove a view.
SQL state: 42809
That is unexpected and also difficult to handle, if I want to be sure that I can delete all old tables *and* views called X, and create a new
view for instance with
CREATE VIEW X AS ....
I cannot do that safely, because having both DROP commands would for sure cause an error and therefore a rollback.
What do you think, is it worth to create a patch to solve this issue, where a DROP TABLE does not fail, if the given name is actually a VIEW or vice-versa?
DROP TABLE should to remove table and nothing else, like DROP VIEW should to drop just view and nothing else. It is safeguard.
what is hard on code
do $$ declare r record;
begin
for r in select table_name, case table_type when 'BASE TABLE' then 'table' when 'VIEW' then 'view' end as tp
from information_schema.tables
where table_type in ('BASE TABLE', 'VIEW')
and table_name = 'foo'
loop
raise notice '%', format('drop %s %I', r.tp, r.table_name);
end loop;
end $$;
replace raise notice by execute if you really want to drop some objects.
Regards
Pavel
Best regards,
Peter
On 06/26/2018 05:34 PM, Pavel Stehule wrote: > 2018-06-26 17:23 GMT+02:00 Peter Moser <pitiz29a@gmail.com > <mailto:pitiz29a@gmail.com>>: > What do you think, is it worth to create a patch to solve this > issue, where a DROP TABLE does not fail, if the given name is > actually a VIEW or vice-versa? > > > DROP TABLE should to remove table and nothing else, like DROP VIEW > should to drop just view and nothing else. It is safeguard. My last sentence is misleading. What I thought is: DROP TABLE X; Should not delete a view X, and result in an error only if table X does not exist. It should not look at views for error handling, maybe just as hint that there is a view X and we might use DROP VIEW X instead. DROP TABLE IF EXISTS X; Should also not delete a view X, and not result in any error if X is a view and not a table. Hopefully I explained things better now. Cheers, Peter
On Tuesday, June 26, 2018, Pavel Stehule <pavel.stehule@gmail.com> wrote:
2018-06-26 17:23 GMT+02:00 Peter Moser <pitiz29a@gmail.com>:Hi,
I want to delete a table X, that may not exist, hence I execute
DROP TABLE IF EXISTS X;
However, if X is a view, I get an error
ERROR: "X" is not a tableHINT: Use DROP VIEW to remove a view.
SQL state: 42809
That is unexpected and also difficult to handleDROP TABLE should to remove table and nothing else, like DROP VIEW should to drop just view and nothing else. It is safeguard.
Peter isn't asking for drop table to drop a view though, he's asking for the documented behavior:
"Do not throw an error if the table does not exist. A notice is issued in this case."
There is no Table named X in the database so the command should be a noop with a notice. I would concur, though I'm open to just fixing it in v12 and back patching a documentation bug fix stating the exception due to relations sharing a namespace but there be lacking a corresponding shared namespace "drop relation" command.
David J.
2018-06-26 17:48 GMT+02:00 David G. Johnston <david.g.johnston@gmail.com>:
On Tuesday, June 26, 2018, Pavel Stehule <pavel.stehule@gmail.com> wrote:2018-06-26 17:23 GMT+02:00 Peter Moser <pitiz29a@gmail.com>:Hi,
I want to delete a table X, that may not exist, hence I execute
DROP TABLE IF EXISTS X;
However, if X is a view, I get an error
ERROR: "X" is not a tableHINT: Use DROP VIEW to remove a view.
SQL state: 42809
That is unexpected and also difficult to handleDROP TABLE should to remove table and nothing else, like DROP VIEW should to drop just view and nothing else. It is safeguard.Peter isn't asking for drop table to drop a view though, he's asking for the documented behavior:"Do not throw an error if the table does not exist. A notice is issued in this case."
This is different issue.
There is no Table named X in the database so the command should be a noop with a notice. I would concur, though I'm open to just fixing it in v12 and back patching a documentation bug fix stating the exception due to relations sharing a namespace but there be lacking a corresponding shared namespace "drop relation" command.
There are two points
a) documentation issue
b) different behave for DROP TABLE IF EXISTS command
My note is related to @b. I understand to the motivation, but I am not sure if it is good idea. Tables and views shares one namespace.
Often usage of DROP TABLE IF EXISTS is together with CREATE TABLE
Now if some does bad reference in DROP TABLE command, then this command fails (first). If we do proposed change, then DROP TABLE do nothing, and CREATE TABLE fails.
So I am not sure, if proposed change is practical because views and tables shares same namespace and current behave has sense too.
Regards
Pavel
David J.
On Tuesday, June 26, 2018, Pavel Stehule <pavel.stehule@gmail.com> wrote:
My note is related to @b. I understand to the motivation, but I am not sure if it is good idea. Tables and views shares one namespace.
But the command say "drop table" and so it must only be concerned with that subset of the namespace when searching. The note about the view is helpful, but it shouldn't change whether the command succeeded with a notice or errors.
Often usage of DROP TABLE IF EXISTS is together with CREATE TABLENow if some does bad reference in DROP TABLE command, then this command fails (first). If we do proposed change, then DROP TABLE do nothing, and CREATE TABLE fails.
Yes, this is what should be happening. CREATE does have to care about the whole namespace since it creating a new identifier.
So I am not sure, if proposed change is practical because views and tables shares same namespace and current behave has sense too.
I'm doubtful that there is any meaningful technical/practical challenge involved here. And as you say when doing paired drop/creates one of them is going to fail anyway so it doesn't really matter which one. But if someone wants to convert a table to a view imdompotently (the point if INE) right now they cannot using the features that are documented to do just that.
David J.
2018-06-26 18:22 GMT+02:00 David G. Johnston <david.g.johnston@gmail.com>:
On Tuesday, June 26, 2018, Pavel Stehule <pavel.stehule@gmail.com> wrote:My note is related to @b. I understand to the motivation, but I am not sure if it is good idea. Tables and views shares one namespace.But the command say "drop table" and so it must only be concerned with that subset of the namespace when searching. The note about the view is helpful, but it shouldn't change whether the command succeeded with a notice or errors.Often usage of DROP TABLE IF EXISTS is together with CREATE TABLENow if some does bad reference in DROP TABLE command, then this command fails (first). If we do proposed change, then DROP TABLE do nothing, and CREATE TABLE fails.Yes, this is what should be happening. CREATE does have to care about the whole namespace since it creating a new identifier.So I am not sure, if proposed change is practical because views and tables shares same namespace and current behave has sense too.I'm doubtful that there is any meaningful technical/practical challenge involved here. And as you say when doing paired drop/creates one of them is going to fail anyway so it doesn't really matter which one. But if someone wants to convert a table to a view imdompotently (the point if INE) right now they cannot using the features that are documented to do just that.
Just I don't see this proposal as clean win. More it is not limited only this case. It should be consistent with DROP INDEX, SEQUENCE, ...
Regards
Pavel
David J.
On Tuesday, June 26, 2018, Pavel Stehule <pavel.stehule@gmail.com> wrote:
Just I don't see this proposal as clean win. More it is not limited only this case. It should be consistent with DROP INDEX, SEQUENCE, ...
Yes, they are likely all broken in the same way and whomever agrees with the "it's bugged" conclusion and is willing and able to write a patch should check them and possibly add those checks as regression tests.
But without a concrete example of problems I'm not going to be convinced there is a downside to fixing this bug. Given it's converting an error into an equivalent end result without the error I'd support a committer who wants to backpatch whatever fix is devised.
David J.
Pavel Stehule <pavel.stehule@gmail.com> writes:
> 2018-06-26 18:22 GMT+02:00 David G. Johnston <david.g.johnston@gmail.com>:
>>> So I am not sure, if proposed change is practical because views and
>>> tables shares same namespace and current behave has sense too.
>> I'm doubtful that there is any meaningful technical/practical challenge
>> involved here.
Certainly we *could* change it, but it's not at all clear that it's a good
idea. The current behavior seemed sensible when it was implemented, and
it has stood for quite some years now. Now, we have one person
complaining that it wasn't what he expected. If we change the behavior on
the strength of that, will we change it back on the first complaint from
someone else? What about the possibility that the change breaks peoples'
applications?
I think there might be grounds for clarifying the documentation, but
I'm quite hesitant to change the behavior without someone making a case
a whole lot stronger than this.
Also worth noting is that similar issues arise elsewhere, eg we now
have "procedures" vs "functions" in a single namespace. Let's not have
DROP TABLE acting in a way that's inconsistent with other parts of the
system.
regards, tom lane
Pavel Stehule <pavel.stehule@gmail.com> writes:
> 2018-06-26 18:22 GMT+02:00 David G. Johnston <david.g.johnston@gmail.com>:
>>> So I am not sure, if proposed change is practical because views and
>>> tables shares same namespace and current behave has sense too.
>> I'm doubtful that there is any meaningful technical/practical challenge
>> involved here.
Certainly we *could* change it, but it's not at all clear that it's a good
idea. The current behavior seemed sensible when it was implemented, and
it has stood for quite some years now.
There are numerous examples of bugs that go unreported until well after a release is out of support; I'm leaning toward figuring this falls into that category. A code comment, commit, or email where this was discussed explicitly would help shift my view here.
Now, we have one person
complaining that it wasn't what he expected. If we change the behavior on
the strength of that, will we change it back on the first complaint from
someone else?
I'm not saying change it because someone was confused but because the wording of the feature leads me to believe that the current implementation is wrong and...
What about the possibility that the change breaks peoples'
applications?
...it is broken in such a way that I cannot foresee someone relying upon the existing behavior intentionally.
I'm willing to admit that this is an edge case and fixing it only in 12 is acceptable. I'm unable to prove a negative here but likewise no one has shown the negative to be false either.
I think there might be grounds for clarifying the documentation, but
I'm quite hesitant to change the behavior without someone making a case
a whole lot stronger than this.
OK.
Also worth noting is that similar issues arise elsewhere, eg we now
have "procedures" vs "functions" in a single namespace. Let's not have
DROP TABLE acting in a way that's inconsistent with other parts of the
system.
Maybe....VIEWS and TABLES are in many ways interchangeable whereas other objects sharing a namespace have distinct uses that are different from each other. For those the choice here doesn't even matter (though if this is how procedures/functions work I wouldn't be surprise if we got this kind of complaint for those sooner rather than later) as no one would be running code where they want to drop a table and create a view of the same name (or vice-versa I suppose). I'm sure it doesn't happen that often, hence the lack of reports, but it also a transformation that I wouldn't consider unreasonable (client code doesn't want to change the logically named relation even though the DBA wants to change the physical implementation).
David J.
On 06/26/2018 07:06 PM, Tom Lane wrote: > Also worth noting is that similar issues arise elsewhere, eg we now > have "procedures" vs "functions" in a single namespace. Let's not have > DROP TABLE acting in a way that's inconsistent with other parts of the > system. I think, that DROP <type> <identifier> ... could search within the type's namespace for the <type><identifier> in combination, and only fail if it cannot be found. I use those commands in a project with an Java ORM in place, that automatically generates/updates a schema on each startup. It wrongly generates a table X, where it should generate a view X. Hence, I do the following inside an sql-script after startup: DROP TABLE X IF EXISTS ... DROP VIEW X IF EXISTS ... CREATE VIEW X ... It works on the first run, but not on a subsequent one, because the view X already exists, hence DROP TABLE X fails. If I switch the first two lines, it fails already during the first run, because a table X exists... DROP VIEW X IF EXISTS ... DROP TABLE X IF EXISTS ... CREATE VIEW X ... It is only solvable with two different calls to the database, and error handling on the application side. Intuitively, I (and also others, that I asked) think that this command should only fail, if a search for <type><identifier> in combination succeeds and the DROP itself fails. In general my use-case is, that I want to delete an X in a certain namespace, where the type is not known in advance. I could query the catalog to get that information and then build a procedure to "execute" the right DROP command (as Pavel Stehule suggested), but that adds complexity to the application code, where it shouldn't be necessary IMHO. Best regards, Peter
On 28 June 2018 at 05:37, Peter Moser <pitiz29a@gmail.com> wrote:
[....]
In general my use-case is, that I want to delete an X in a certain namespace, where the type is not known in advance. I could query the catalog to get that information and then build a procedure to "execute" the right DROP command (as Pavel Stehule suggested), but that adds complexity to the application code, where it shouldn't be necessary IMHO.
I've dealt with this issue in some contexts by writing a procedure which takes a regclass parameter and formats and executes an appropriate "DROP [x]" command.
On a related note, I sometimes find myself wanting to drop a bunch of tables and views and I find it inconvenient that I have to split up my drop into two commands - one for the views and one for the tables.
This is a vote for a "DROP RELATION" command that doesn't care if the objects are views, materialized views, tables, or a mix of those. Maybe even index or sequence or the other possible values of pg_class.relkind, although I don't normally think of those as relations.
On Tue, Jun 26, 2018 at 1:06 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Certainly we *could* change it, but it's not at all clear that it's a good > idea. The current behavior seemed sensible when it was implemented, and > it has stood for quite some years now. Now, we have one person > complaining that it wasn't what he expected. If we change the behavior on > the strength of that, will we change it back on the first complaint from > someone else? What about the possibility that the change breaks peoples' > applications? Hmm. I think we generally allow ALTER TABLE to work on non-table relations. For example, ALTER TABLE .. OWNER TO happily accepts a view or materialized view as an argument. I have had my doubts about that policy from time to time (cf. 1489e2f26a4c0318938b3085f50976512f321d84) but it does seem to be the policy (cf. b14206862278347a379f2bb72d92d16fb9dcea45). Is there some reason to think that the same policy that we apply to ALTER should not also apply to DROP? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> On Tue, Jun 26, 2018 at 1:06 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Certainly we *could* change it, but it's not at all clear that it's a good
>> idea. The current behavior seemed sensible when it was implemented, and
>> it has stood for quite some years now. Now, we have one person
>> complaining that it wasn't what he expected. If we change the behavior on
>> the strength of that, will we change it back on the first complaint from
>> someone else? What about the possibility that the change breaks peoples'
>> applications?
> Hmm. I think we generally allow ALTER TABLE to work on non-table
> relations. For example, ALTER TABLE .. OWNER TO happily accepts a
> view or materialized view as an argument. I have had my doubts about
> that policy from time to time (cf.
> 1489e2f26a4c0318938b3085f50976512f321d84) but it does seem to be the
> policy (cf. b14206862278347a379f2bb72d92d16fb9dcea45). Is there some
> reason to think that the same policy that we apply to ALTER should not
> also apply to DROP?
Uh ... error proofing? The thing about ALTER is that if you mistakenly
point at the wrong object, it's quite likely your command will fail
because the properties of the object don't match (eg, it doesn't have a
column by that name). Or if it goes through anyway, many varieties of
ALTER are reversible -- certainly ALTER OWNER is. There's no comparable
safety margin with DROP, and it's not reversible, and the consequences of
a mistake could be pretty bad.
Note that in any case this wasn't what the OP was proposing, if I
understood that correctly.
regards, tom lane
On Mon, Jul 2, 2018 at 5:25 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> On Tue, Jun 26, 2018 at 1:06 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> Certainly we *could* change it, but it's not at all clear that it's a good >>> idea. The current behavior seemed sensible when it was implemented, and >>> it has stood for quite some years now. Now, we have one person >>> complaining that it wasn't what he expected. If we change the behavior on >>> the strength of that, will we change it back on the first complaint from >>> someone else? What about the possibility that the change breaks peoples' >>> applications? > >> Hmm. I think we generally allow ALTER TABLE to work on non-table >> relations. For example, ALTER TABLE .. OWNER TO happily accepts a >> view or materialized view as an argument. I have had my doubts about >> that policy from time to time (cf. >> 1489e2f26a4c0318938b3085f50976512f321d84) but it does seem to be the >> policy (cf. b14206862278347a379f2bb72d92d16fb9dcea45). Is there some >> reason to think that the same policy that we apply to ALTER should not >> also apply to DROP? > > Uh ... error proofing? The thing about ALTER is that if you mistakenly > point at the wrong object, it's quite likely your command will fail > because the properties of the object don't match (eg, it doesn't have a > column by that name). Or if it goes through anyway, many varieties of > ALTER are reversible -- certainly ALTER OWNER is. There's no comparable > safety margin with DROP, and it's not reversible, and the consequences of > a mistake could be pretty bad. I suppose that's true, but it still seems inconsistent. > Note that in any case this wasn't what the OP was proposing, if I > understood that correctly. Yes, the original proposal was that we should be relaxed about it. Another possibility that would also seem to meet the OP's needs is to make it do this: DROP TABLE IF EXISTS X; NOTICE: relation "X" is not a table, skipping His complaint was really that it generated an ERROR, IIUC. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Mon, Jul 2, 2018 at 10:33 PM, Robert Haas <robertmhaas@gmail.com> wrote: > Yes, the original proposal was that we should be relaxed about it. ...in both directions i.e. DROP TABLE would work on a VIEW and DROP VIEW on a table. That definitely seems like it's going too far. > Another possibility that would also seem to meet the OP's needs is to > make it do this: > > DROP TABLE IF EXISTS X; > NOTICE: relation "X" is not a table, skipping > > His complaint was really that it generated an ERROR, IIUC. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> Another possibility that would also seem to meet the OP's needs is to
> make it do this:
> DROP TABLE IF EXISTS X;
> NOTICE: relation "X" is not a table, skipping
> His complaint was really that it generated an ERROR, IIUC.
While that would perhaps meet the OP's desires, it still seems like
a strange definition of "IF EXISTS" to me. That's supposed to be an
escape hatch for "object does not exist", not for other error types.
The long and short of it is that I'm not dissatisfied with the way
this works now, and given the lack of previous complaints, not
many other people are either. So I do not think this is a bug fix;
it's a definition disagreement. I'm inclined to think that the
principle of "stare decisis" ought to apply here --- once we've let
a particular behavior stand for N release cycles, it should take
more than one or two people objecting to change it, because
backwards compatibility.
Also, based on other messages, it seems like what the OP wants is
to be sure that "CREATE TABLE X" will succeed afterwards, so that
failing to get rid of view X will not do what he needs.
There might be some merit in pursuing the DROP RELATION idea that
was floated earlier. That seems analogous to DROP ROUTINE, which
is dealing with a related case and apparently is standards-compliant.
regards, tom lane
On Wednesday, July 4, 2018, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Also, based on other messages, it seems like what the OP wants is
to be sure that "CREATE TABLE X" will succeed afterwards, so that
failing to get rid of view X will not do what he needs.
I read and agree that what should be possible, absent DROP RELATION, is for someone to execute both DROP VIEW and DROP TABLE with the same name, in any order, and not have the transaction abort with an error (if a table or view of the same name already exists). Having done that the CREATE VIEW will succeed since either both were no-ops (no table or view of that name existed) or one was guaranteed to succeed and the other was a no-op (table or view existed - doesn't matter which). As it stands now you have to know whether the existing object is a table or a view in order to get the order correct and the first run usually the table exists and the second run the view exists so the script has to be changed in between.
If this didn't involve an error mode the desire to leave things as-is would be more understandable to me.
David J.
2018-07-04 23:17 GMT+02:00 Tom Lane <tgl@sss.pgh.pa.us>:
Robert Haas <robertmhaas@gmail.com> writes:
> Another possibility that would also seem to meet the OP's needs is to
> make it do this:
> DROP TABLE IF EXISTS X;
> NOTICE: relation "X" is not a table, skipping
> His complaint was really that it generated an ERROR, IIUC.
While that would perhaps meet the OP's desires, it still seems like
a strange definition of "IF EXISTS" to me. That's supposed to be an
escape hatch for "object does not exist", not for other error types.
The long and short of it is that I'm not dissatisfied with the way
this works now, and given the lack of previous complaints, not
many other people are either. So I do not think this is a bug fix;
it's a definition disagreement. I'm inclined to think that the
principle of "stare decisis" ought to apply here --- once we've let
a particular behavior stand for N release cycles, it should take
more than one or two people objecting to change it, because
backwards compatibility.
Also, based on other messages, it seems like what the OP wants is
to be sure that "CREATE TABLE X" will succeed afterwards, so that
failing to get rid of view X will not do what he needs.
There might be some merit in pursuing the DROP RELATION idea that
was floated earlier. That seems analogous to DROP ROUTINE, which
is dealing with a related case and apparently is standards-compliant.
+1 DROP ROUTINE proposal
Regards
Pavel
regards, tom lane