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
