Re: DROP DATABASE is interruptible

From: Andres Freund <andres(at)anarazel(dot)de>
To: Daniel Gustafsson <daniel(at)yesql(dot)se>
Cc: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Evgeny Morozov <postgresql3(at)realityexists(dot)net>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Subject: Re: DROP DATABASE is interruptible
Date: 2023-07-13 20:52:33
Message-ID: 20230713205233.qwwk5heqktvufs7y@awork3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
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(at)anarazel(dot)de> wrote:
> > On 2023-07-07 14:09:08 +0200, Daniel Gustafsson wrote:
> >>> On 25 Jun 2023, at 19:03, Andres Freund <andres(at)anarazel(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Gurjeet Singh 2023-07-13 21:00:29 Re: Fix search_path for all maintenance commands
Previous Message Andres Freund 2023-07-13 20:45:41 Re: vac_truncate_clog()'s bogus check leads to bogusness