MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Date: Thu, 03 Apr 2025 14:34:27 +0900
From: torikoshia <torikoshia@oss.nttdata.com>
To: Sadeq Dousti <msdousti@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org, Sergey Dudoladov
 <sergey.dudoladov@gmail.com>
Subject: Re: RFC: Logging plan of the running query
In-Reply-To: 
 <CADE6Lvi1yJYF8KgK9ofin50bRKxByqSHufxshj0pLC7dfoZD9g@mail.gmail.com>
References: <d68c3ae31672664876b22d2dcbb526d2@postgrespro.ru>
 <2aedcb5e8120bfdee994775e660f8718@oss.nttdata.com>
 <174144854199.294105.9417032645816235284.pgcf@coridan.postgresql.org>
 <ffdfab063ac6f3d3d97763c9d744b70b@oss.nttdata.com>
 <784224198089cbe3cc3825332f195475@oss.nttdata.com>
 <CADE6Lvi1yJYF8KgK9ofin50bRKxByqSHufxshj0pLC7dfoZD9g@mail.gmail.com>
User-Agent: Roundcube Webmail/1.4.11
Message-ID: <e04b16ddfad83bab3632617e8cda9b42@oss.nttdata.com>
Archived-At: 
 <https://www.postgresql.org/message-id/e04b16ddfad83bab3632617e8cda9b42%40oss.nttdata.com>
Precedence: bulk

On 2025-04-01 03:51, Sadeq Dousti wrote:
> Hi,
> 
> Sergey and I reviewed this patch and have a few comments, listed
> below.
Thanks for your review!

>> BTW the patch adds about 400 lines to explain.c and it may be better
>> to split the file as well as 9173e8b6046, but I leave it as it is
> for
>> now.
> 
> 1. As rightfully described by the OP above, explain.c has grown too
> big. In addition, explain.h is added to quite a number of other files.
Agreed.

> 
> 2. We wanted to entertain the possibility of a GUC variable to enable
> the feature. That is, having it disabled by default, and the DBA can
> selectively enable it on the fly. The reasoning is that it can affect
> some workloads in an unforeseen way, so this choice can make the next
> Postgres release safer. In future releases, we can make the default
> "enabled", assuming enough real-world scenarios have proven the
> feature safe (i.e., not affecting the DB performance noticeably).
Agreed.

> 3. In the email thread for this patch, we saw some attempts to measure
> the performance overhead of the feature. We suggest a more rigorous
> study, including memory overhead in addition to time overhead. We came
> to the conclusion that analytical workloads - with complicated query
> plans - and a high query rate are the best to attempt such benchmarks.
Agreed.

> 4. In PGConf EU 2024, there was a talk by Rafael Castro titled
> "Debugging active queries" [1], and he also submitted a recent patch
> titled "Proposal: Progressive explain" [2]. We see a lot of
> similarities in the idea behind that patch and this one, and would
> like to suggest joining forces for an ideal outcome.
As we have been discussing in this thread recently, I believe that since 
this patch has a narrower scope, it makes sense to complete it first and 
then extend it to support progress tracking in a step-by-step manner.

-- 
Regards,

--
Atsushi Torikoshi
Seconded from NTT DATA GROUP CORPORATION to SRA OSS K.K.