Re: DROP DATABASE is interruptible
Andres Freund <andres@anarazel.de>
From: Andres Freund <andres@anarazel.de>
To: Daniel Gustafsson <daniel@yesql.se>
Cc: Thomas Munro <thomas.munro@gmail.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>, Evgeny Morozov <postgresql3@realityexists.net>, Tom Lane <tgl@sss.pgh.pa.us>
Date: 2023-07-13T20:52:33Z
Lists: pgsql-hackers
Hi,
On 2023-07-12 11:54:18 +0200, Daniel Gustafsson wrote:
> > On 12 Jul 2023, at 03:59, Andres Freund <andres@anarazel.de> wrote:
> > On 2023-07-07 14:09:08 +0200, Daniel Gustafsson wrote:
> >>> On 25 Jun 2023, at 19:03, Andres Freund <andres@anarazel.de> wrote:
> >>> On 2023-06-21 12:02:04 -0700, Andres Freund wrote:
>
> >>> There don't need to be explict checks, because pg_upgrade will fail, because
> >>> it connects to every database. Obviously the error could be nicer, but it
> >>> seems ok for something hopefully very rare. I did add a test ensuring that the
> >>> behaviour is caught.
> >>
> >> I don't see any pg_upgrade test in the patch?
> >
> > Oops, I stashed them alongside some unrelated changes... Included this time.
>
> Looking more at this I wonder if we in HEAD should make this a bit nicer by
> extending the --check phase to catch this? I did a quick hack along these
> lines in the 0003 commit attached here (0001 and 0002 are your unchanged
> patches, just added for consistency and to be CFBot compatible). If done it
> could be a separate commit to make the 0002 patch backport cleaner of course.
I don't really have an opinion on that, tbh...
> >> + errhint("Use DROP DATABASE to drop invalid databases"));
> >> Should end with a period as a complete sentence?
> >
> > I get confused about this every time. It's not helped by this example in
> > sources.sgml:
> >
> > <programlisting>
> > Primary: could not create shared memory segment: %m
> > Detail: Failed syscall was shmget(key=%d, size=%u, 0%o).
> > Hint: the addendum
> > </programlisting>
> >
> > Which notably does not use punctuation for the hint. But indeed, later we say:
> > <para>
> > Detail and hint messages: Use complete sentences, and end each with
> > a period. Capitalize the first word of sentences. Put two spaces after
> > the period if another sentence follows (for English text; might be
> > inappropriate in other languages).
> > </para>
>
> That's not a very helpful example, and one which may give the wrong impression
> unless the entire page is read. I've raised this with a small diff to improve
> it on -docs.
Thanks for doing that!
> > Updated patches attached.
>
> This version of the patchset LGTM.
Backpatching indeed was no fun. Not having BackgroundPsql.pm was the worst
part. But also a lot of other conflicts in tests... Took me 5-6 hours or
so.
But I now finally pushed the fixes. Hope the buildfarm agrees with it...
Thanks for the review!
Greetings,
Andres Freund
Commits
-
pg_dump: tests: Correct test condition for invalid databases
- 881defde944c 16.1 landed
- 6a3b19bbefd7 11.22 landed
- fd7a114dbb4e 12.17 landed
- d1c76fdec375 13.13 landed
- 4dfb610822d5 14.10 landed
- 9dc3c5472ed9 15.5 landed
- 849d367ff9a2 17.0 landed
-
Handle DROP DATABASE getting interrupted
- f66403749df7 15.4 landed
- 1c38e7ae17b6 11.21 landed
- 034a9fcd2bb8 12.16 landed
- 81ce000067e3 13.12 landed
- d11efe830385 14.9 landed
- a4b4cc1d60f7 16.0 landed
- c66a7d75e652 17.0 landed