public inbox for [email protected]  
help / color / mirror / Atom feed
From: Michael Paquier <[email protected]>
To: Drouvot, Bertrand <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Cc: [email protected]
Cc: [email protected]
Subject: Re: WIP: new system catalog pg_wait_event
Date: Wed, 9 Aug 2023 16:56:27 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>

On Tue, Aug 08, 2023 at 10:16:37AM +0200, Drouvot, Bertrand wrote:
> Please find attached v3 adding the wait event types.

+-- There will surely be at least 9 wait event types, 240 wait events and at
+-- least 27 related to WAL
+select count(distinct(wait_event_type)) > 8 as ok_type,
+       count(*) > 239 as ok,
+       count(*) FILTER (WHERE description like '%WAL%') > 26 AS ok_wal_desc
+from pg_wait_event;
The point is to check the execution of this function, so this could be
simpler, like that or a GROUP BY clause with the event type:
SELECT count(*) > 0 FROM pg_wait_event;
SELECT wait_event_type, count(*) > 0 AS has_data FROM pg_wait_event
  GROUP BY wait_event_type ORDER BY wait_event_type;

+           printf $ic "\tmemset(values, 0, sizeof(values));\n";
+           printf $ic "\tmemset(nulls, 0, sizeof(nulls));\n\n";
+           printf $ic "\tvalues[0] = CStringGetTextDatum(\"%s\");\n", $last;
+           printf $ic "\tvalues[1] = CStringGetTextDatum(\"%s\");\n", $wev->[1];
+           printf $ic "\tvalues[2] = CStringGetTextDatum(\"%s\");\n\n", $new_desc;

That's overcomplicated for some code generated.  Wouldn't it be
simpler to generate a list of elements, with the code inserting the
tuples materialized looping over it?

+           my $new_desc = substr $wev->[2], 1, -2;
+           $new_desc =~ s/'/\\'/g;
+           $new_desc =~ s/<.*>(.*?)<.*>/$1/g;
+           $new_desc =~ s/<xref linkend="guc-(.*?)"\/>/$1/g;
+           $new_desc =~ s/; see.*$//;
Better to document what this does, the contents produced look good.

+   rename($ictmp, "$output_path/pg_wait_event_insert.c")
+     || die "rename: $ictmp to $output_path/pg_wait_event_insert.c: $!";

 # seems nicer to not add that as an include path for the whole backend.
 waitevent_sources = files(
   'wait_event.c',
+  'pg_wait_event.c',
 )

This could use a name referring to SQL functions, say
wait_event_funcs.c, with a wait_event_data.c or a
wait_event_funcs_data.c?

+       # Don't generate .c (except pg_wait_event_insert.c) and .h files for
+       # Extension, LWLock and Lock, these are handled independently.
+       my $is_exception = $waitclass eq 'WaitEventExtension' ||
+                          $waitclass eq 'WaitEventLWLock' ||
+                          $waitclass eq 'WaitEventLock';
Perhaps it would be cleaner to use a separate loop?
--
Michael


Attachments:

  [application/pgp-signature] signature.asc (833B, ../[email protected]/2-signature.asc)
  download

view thread (7+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: WIP: new system catalog pg_wait_event
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox