On Sun, Jun 16, 2024 at 09:28:05AM +0900, Michael Paquier wrote:
> On Thu, Jun 13, 2024 at 07:42:25PM -0700, Noah Misch wrote:
> > On Fri, Jun 14, 2024 at 09:58:59AM +0900, Michael Paquier wrote:
> >> GetWaitEventCustomIdentifier() is incorrect, and should return
> >> "InjectionPoint" in the default case of this class name, no?
> >
> > I intentionally didn't provide a default event ID for InjectionPoint.
> > PG_WAIT_EXTENSION needs a default case for backward compatibility, if nothing
> > else. For this second custom type, it's needless complexity. The value
> > 0x0B000000U won't just show up like PG_WAIT_EXTENSION does.
> > GetLWLockIdentifier() also has no default case. How do you see it?
>
> I would add a default for consistency as this is just a few extra
> lines, but if you feel strongly about that, I'm OK as well. It makes
> a bit easier the detection of incorrect wait event numbers set
> incorrectly in extensions depending on the class wanted.
It would be odd to detect exactly 0x0B000000U and not other invalid inputs,
like 0x0A000001U where only 0x0B000001U is valid. I'm attaching roughly what
it would take. Shall I squash this into inplace031?
The thing I feel strongly about here is keeping focus on fixing $SUBJECT bugs
that are actually corrupting data out there. I think we should all limit our
interest in the verbiage of strings that appear only when running developer
tests, especially when $SUBJECT is a bug fix. When the string appears only
after C code passes invalid input to other C code, it matters even less.
> > The patch added to xfunc.sgml an example of using it. I'd be more inclined to
> > delete the WaitEventExtensionNew() docbook documentation than to add its level
> > of detail for WaitEventInjectionPointNew(). We don't have that kind of
> > documentation for most extension-facing C functions.
>
> It's one of the areas where I think that we should have more
> documentation, not less of it, so I'd rather keep it and maintaining
> it is not really a pain (?). The backend gets complicated enough
> these days that limiting what developers have to guess on their own is
> a better long-term approach because the Postgres out-of-core ecosystem
> is expanding a lot (aka have also in-core documentation for hooks,
> even if there's been a lot of reluctance historically about having
> them).
[getting deeply off topic -- let's move this to another thread if it needs to
expand] I like reducing the need to guess. So far in this inplace update
project (this thread plus postgr.es/m/20240615223718.42.nmisch@google.com),
three patches just fix comments. Even comments carry quite a price, but I
value them. When we hand-maintain documentation of a C function in both its
header comment and another place, I get skeptical about whether hackers
(including myself) will actually keep them in sync and skeptical of the
incremental value of maintaining the second version.
