Hi,
On 4/24/23 5:15 AM, Michael Paquier wrote:
> On Sat, Apr 22, 2023 at 03:36:05PM +0200, Drouvot, Bertrand wrote:
>> On 4/20/23 3:09 AM, Michael Paquier wrote:
>>> It is clear that the format of the file is:
>>> - category
>>> - C symbol in enums.
>>> - Format in the system views.
>>> - Description in the docs.
>>> Or perhaps it would be better to divide this file by sections (like
>>> errcodes.txt) for each category so as we eliminate entirely the first
>>> column?
>>
>> Yeah, that could be an option. V3 is still using the category as the first column
>> but I'm ok to change it by a section if you prefer (though I don't really see the need).
>
> It can make the file width shorter, at least..
Right.
>
> [ .. thinks .. ]
>
> +my $waitclass;
> +my @wait_classes = ("PG_WAIT_ACTIVITY", "PG_WAIT_CLIENT", "PG_WAIT_IPC", "PG_WAIT_TIMEOUT", "PG_WAIT_IO");
>
> Actually, having a "Section" in waiteventnames.txt would remove the
> need to store this knowledge in generate-waiteventnames.pl, which is
> a duplicate of the txt contents. If somebody adds a new class in the
> future, it would be necessary to update this path as well. Well, that
> would not be a huge effort in itself, but IMO the script translating
> the .txt to the docs and the code should have no need to know the
> types of classes. I guess that a format like that would make the most
> sense to me, then:
> Section: ClassName PG_WAIT_CLASS_NAME
>
> # ClassName would be "IO", "IPC", "Timeout", etc.
>
> WAIT_EVENT_NAME_1 "WaitEventName1" "Description of wait event 1"
> WAIT_EVENT_NAME_N "WaitEventNameN" "Description of wait event N"
>
I gave another thought on it, and do agree that's better to use sections
in the .txt file. This is done in V4 attached.
>>> utils/adt/jsonpath_scan.c \
>>> + utils/activity/waiteventnames.c \
>>> + utils/activity/waiteventnames.h \
>>> + utils/adt/jsonpath_scan.c \
>>>
>>> Looks like a copy-pasto.
>>
>> Why do you think so? both files have to be removed.
>
> jsonpath_scan.c is listed twice, and that's still the case in v3.
Oh I see, I misunderstood what you thought the typo was.
Fixed in V4 thanks!
> The
> list of files deleted for maintainer-clean in src/backend/Makefile
> should be listed alphabetically (utils/activity/ before utils/adt/),
> but that's a nit ;)
Oh right, fixed.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
