Re: dropdb --force

čt 25. 7. 2019 v 5:11 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:

> Pavel Stehule <pavel.stehule@gmail.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().
>
> * 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 ...
>
>
Can be

DROP DATABASE '(' options ...) [IF EXISTS] name

ok?


>
> 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.
>

I don't think so server side implementation is too helpful - there is lot
of situations, where DDL command is much more practical.


>                         regards, tom lane
>