Re: Reorder shutdown sequence, to flush pgstats later

Hi,

On Wed, Jan 08, 2025 at 02:32:24PM -0500, Andres Freund wrote:
> On 2025-01-08 14:48:21 +0000, Bertrand Drouvot wrote:
> > === 2
> >
> > + default:
> > + /* all signals sent by postmaster should be listed here */
> > + Assert(false);
> > + return "(unknown)";
> > + }
> > +#undef PM_CASE
> > + pg_unreachable();
> >
> > Shouldn't we remove the "return "(unknown)"? (if not the pg_unreachable() looks
> > like dead code).
>
> I don't think so - we're not listing all signals here, just ones that
> postmaster is currently using. They're also typically not defined in an enum
> allowing the compiler to warn about uncovered values.

Oh right, the parameter is an "int" not an "enum" (and anyway, as you said, we're
not listing all the signals) (did not pay attention to that). So we may need to
"help" some compilers regarding missing return values.

> It seemed better to
> actually return something in a production build rather than aborting.

Yeah, fully agree.

> Perhaps I should have done a
> return "" /* silence compilers */;

Using pg_unreachable() is fine but a comment (like the return value you're
suggesting above) could help.

Thanks for the explanation!

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com