Re: dropdb --force

Hi

I started work on this patch. I changed syntax to

DROP DATABASE [ ( FORCE , ..) ] [IF EXISTS ...]

and now I try to fix all other points from Tom's list

út 17. 9. 2019 v 12:15 odesílatel Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> napsal:

> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
> > [ drop-database-force-20190708.patch ]
>
> I took a brief look at this, but I don't think it's really close to
> being committable.
>
> * The documentation claims FORCE will fail if you don't have privileges
> to terminate the other session(s) in the target DB. This is a lie; the
> code issues kill() without any regard for such niceties. You could
> perhaps make that better by going through pg_signal_backend().
>

This is question. There are two possible requirements about necessary rights

a) rights for other process termination
b) database owner can drop database

I understand very well to Tom's objection, but request to have
ROLE_SIGNAL_BACKEND or be super user looks too strong and not too much
practical.

If I am a owner of database, and I have a right to drop this database, why
I cannot to kick some other user that block database dropping.

What do you think about it? If I use pg_signal_backend, then the necessary
requirement for using DROP FORCE have to be granted SIGNAL_BACKEND rights.

I am sending updated version

Regards

Pavel

>
> * You've hacked CountOtherDBBackends to do something that's completely
> outside the charter one would expect from its name, and not even
> bothered to update its header comment. This requires more attention
> to not confusing future hackers; I'd say you can't even use that
> function name anymore.
>
> * You've also ignored the existing code in CountOtherDBBackends
> that is careful *not* to issue a kill() while holding the critical
> ProcArrayLock. That problem would get enormously worse if you
> tried to sub in pg_signal_backend() there, since that function
> may do catalog accesses --- it's pretty likely that you could
> get actual deadlocks, never mind just trashing performance.
>
> * I really dislike the addition of more hard-wired delays and
> timeouts to dropdb(). It's bad enough that CountOtherDBBackends
> has got that. Two layers of timeout are a rickety mess, and
> who's to say that a 1-minute timeout is appropriate for anything?
>
> * I'm concerned that the proposed syntax is not future-proof.
> FORCE is not a reserved word, and we surely don't want to make
> it one; but just appending it to the end of the command without
> any decoration seems like a recipe for problems if anybody wants
> to add other options later. (Possible examples: RESTRICT/CASCADE,
> or a user-defined timeout.) Maybe some parentheses would help?
> Or possibly I'm being overly paranoid, but ...
>
>
> I hadn't been paying any attention to this thread before now,
> but I'd assumed from the thread title that the idea was to implement
> any attempted kills in the dropdb app, not on the backend side.
> (As indeed it looks like the first version did.) Maybe it would be
> better to go back to that, instead of putting dubious behaviors
> into the core server.
>
> regards, tom lane
>
>
>
>
>