public inbox for [email protected]  
help / color / mirror / Atom feed
From: Rafael Thofehrn Castro <[email protected]>
To: torikoshia <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Atsushi Torikoshi <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Subject: Re: RFC: Logging plan of the running query
Date: Fri, 19 Sep 2025 14:03:57 -0300
Message-ID: <CAG0ozMph-+tx1FnP1FMoaMLfXWUbLBxnALZo6ZQEe9jWMnKo_Q@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<174144854199.294105.9417032645816235284.pgcf@coridan.postgresql.org>
	<[email protected]>
	<[email protected]>
	<CA+TgmoatMzMedKRYAK5s3euxUohd6m00szL4Q6GGQj+NhF813A@mail.gmail.com>
	<[email protected]>
	<CA+TgmoZNYetpFSgVXpE0UdH_J7cPS=8UxxzuV6ubWWwX-8Gpag@mail.gmail.com>
	<CAM6-o=BecZ-FOOHZXLhMU=JGpGQBPS5cA=ASoLUr92fFgjvQqg@mail.gmail.com>
	<[email protected]>
	<CAMT0RQTSGrjqEooQKEJ4X1oQ-OUAw8Zw0fKjSANRkscV0cc0yQ@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<CA+TgmoYFPbMX7DDuXnurJ1OOXoBpN4WbQg6SNubPPSNKtsRWiA@mail.gmail.com>
	<CAM6-o=DJ=ty7pvrddhNWKsnBA3vhBjfw6B=E3asdJ6E_-OLQmA@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<CA+TgmoYBLNSNGSumcDZB4ymVfy-aHyFa_i4n9MHRpPOLp5m+tQ@mail.gmail.com>
	<[email protected]>
	<CA+TgmoYmWZ-PjEkhu6BJj=BGFtvKdfEE3up9e4nvfCLupE9h3A@mail.gmail.com>
	<[email protected]>

Hi folks,

apologies for not replying earlier. Thanks torikoshia for having
reviewed the related patch I sent that includes instrumentation.
It makes total sense to focus on this one first.

Been thinking about the current strategy of having to iterate through
the execution tree to add the custom ExecProcNode to avoid logging
the plan in CHECK_FOR_INTERRUPTS(). I see you folks are still
discussing all the nuances in the recursive tree traversal function.

Taking a step back and proposing a different approach, have we thought
about logging the query plan in the regions of the code related to the
query executor, NEXT to CHECK_FOR_INTERRUPTS calls instead of IN
that function?

For example, in this part of executor/nodeHashjoin.c:

for (;;)
{
  /*
   * It's possible to iterate this loop many times before returning a
   * tuple, in some pathological cases such as needing to move much of
   * the current batch to a later batch.  So let's check for interrupts
   * each time through.
   */
  CHECK_FOR_INTERRUPTS();

We replace CHECK_FOR_INTERRUPTS() for a new function that does
query plan logging logic + CHECK_FOR_INTERRUPTS().

Would that be considered a safe operation given that we would always be
in the query execution context? The current ExecProcNode wrapper strategy
logs the query plan in ExecProcNodeFirst(). That function will then call
the real ExecProcNode, which points to one of the functions that contain
the CHECK_FOR_INTERRUPTS in the first place. I don't see much difference
in terms of logging safety between the two approaches, but maybe there
is :)

Rafael.


view thread (127+ 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], [email protected], [email protected]
  Subject: Re: RFC: Logging plan of the running query
  In-Reply-To: <CAG0ozMph-+tx1FnP1FMoaMLfXWUbLBxnALZo6ZQEe9jWMnKo_Q@mail.gmail.com>

* 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