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

Поиск
Список
Период
Сортировка
От David G. Johnston
Тема Re: DROP relation IF EXISTS Docs and Tests - Bug Fix
Дата
Msg-id CAKFQuwZ8+K7mvntU6sP2cSBe-iOOA7nX-O3ULYYLZWFFbNhPKA@mail.gmail.com
обсуждение исходный текст
Ответ на Re: DROP relation IF EXISTS Docs and Tests - Bug Fix  (Tom Lane <tgl@sss.pgh.pa.us>)
Список pgsql-hackers
On Wed, Jun 17, 2020 at 4:32 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
"David G. Johnston" <david.g.johnston@gmail.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.



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.

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Tom Lane
Дата:
Сообщение: Re: DROP relation IF EXISTS Docs and Tests - Bug Fix
Следующее
От: Melanie Plageman
Дата:
Сообщение: Re: Improve planner cost estimations for alternative subplans