Hi,
On Tue, Jan 02, 2024 at 10:49:03PM -0500, Robert Haas wrote:
> On Tue, Jan 2, 2024 at 4:45 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
> > That seems to date back to commit 14a9101. I can agree that the suffix is
> > somewhat redundant since these are already marked as type "LWLock", but
> > I'll admit I've been surprised by this before, too. IMHO it makes this
> > proposed test more important because you can't just grep for a different
> > lock to find all the places you need to update.
>
> I agree. I am pretty sure that the reason this happened in the first
> place is that I grepped for the name of some other LWLock and adjusted
> things for the new lock at every place where that found a hit.
>
> > > - Check in both directions instead of just one?
> > >
> > > - Verify ordering?
> >
> > To do those things, I'd probably move the test to one of the scripts that
> > generates the documentation or header file (pg_wait_events doesn't tell us
> > whether a lock is predefined or what order it's listed in). That'd cause
> > failures at build time instead of during testing, which might be kind of
> > nice, too.
>
> Yeah, I think that would be better.
+1 to add a test and put in a place that would produce failures at build time.
I think that having the test in the script that generates the header file is more
appropriate (as building the documentation looks less usual to me when working on
a patch).
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com