čt 25. 7. 2019 v 5:11 odesílatel Tom Lane <t...@sss.pgh.pa.us> napsal:
> Pavel Stehule <pavel.steh...@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 >