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. > 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). -- Michael
signature.asc
Description: PGP signature
