Re: DROP relation IF EXISTS Docs and Tests - Bug Fix

On Wed, Jun 17, 2020 at 4:32 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> "David G. Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com> writes:
> > I'm firmly of the belief that the existing behavior of DROP relation IF
> > EXISTS is flawed - it should not be an error if there is a namespace
> > collision but the relkind of the existing relation doesn't match the
> > relkind set by the DROP command.
>
>
The other thread:

https://www.postgresql.org/message-id/CAKFQuwY90%3DGSX_65cYdAm18TWCv4CvnPdHCuH92qfzKSYaFnxQ%40mail.gmail.com

I don't particularly agree, as I said in the other thread. The core
> point here is that it's not clear to me why the specific error of
> "wrong relkind" deserves a pass, while other errors such as "you're
> not the owner" don't.

Because if you're not the owner then by definition the expected target
exists and a drop is attempted - which can still fail.

Both of those cases suggest that you're not
> targeting the relation you think you are, and both of them would get
> in the way of a subsequent CREATE.

Agreed, as noted on the other thread we actually are not sufficiently
paranoid in this situation. Specifically, we allow dropping a relation
based upon a search_path search when the target it not on the first entry
in the search_path. I'd be glad to see that hole closed up - but this is
still broken even when the name is always schema qualified.

To me, success of DROP IF EXISTS
> should mean "the coast is clear to do a CREATE". With an exception
> like this, a success would mean nothing at all.
>

To me and at least some users DROP IF EXISTS means that the specific object
I specified no longer exists, period.

If you want access to the behavior you describe go and write DROP ROUTINE.
As noted on the other thread I think that is a bad option but hey, it does
have the benefit of doing exactly what you describe.

Users can write multiple the drop commands necessary to get their create
command to execute successfully. If the create command fails they can
react to that and figure out where their misunderstanding was. Is that
really so terrible?

Another point here is that we have largely the same issue with respect
> to different subclasses of routines (functions/procedures/aggregates)
> and types (base types/composite types/domains). If we do change
> something then I'd want to see it done consistently across all these
> cases.

Ok. I don't necessarily disagree. In fact the patch I submitted, which is
the on-topic discussion for this thread, brings up the very point that
domain behavior here is presently inconsistent.

At least for DROP TABLE IF EXISTS if we close up the hole with search_path
resolution by introducing an actual "found relation in the wrong location"
error then the risk will have been removed - which exists outside of the IF
EXISTS logic - and instead of not dropping a table and throwing an error we
just are not dropping a table.

So, in summary, this thread is to document the current behavior [actual doc
bug fix]. There is probably another thread buried in all of this for going
through and finding other undocumented behaviors for other object types
[potential doc bug fixes]. Then a thread for solidifying search_path
handling to actually fill in missing seemingly desirable safety features to
avoid drop target mis-identification (so we don't actually drop the wrong
object) [feature]. Then a thread to discuss whether or not dropping an
object that wasn't of the relkind that user specified should be an error
[bug fix held up due to insufficient safety features]. Then a thread to
discuss DROP ROUTINE [user choice of convenience over safety].

David J.