Re: RFC: Logging plan of the running query

On 2025-04-01 21:32, Robert Haas wrote:
Thanks for your comment.

> On Mon, Mar 31, 2025 at 9:43 PM torikoshia <torikoshia@oss.nttdata.com> 
> wrote:
>> Previously, Rafael proposed a patch in this thread that added 
>> execution
>> progress tracking. However, I felt that expanding the scope could make
>> it harder to get the patch reviewed or committed. So, I suggested 
>> first
>> completing a feature that only retrieves the execution plan of a 
>> running
>> query, and then developing execution progress tracking afterward[3].
> 
> That's reasonable. Comparing the two patches, I think that you have
> correctly solved a couple of key problems that Rafael did not handle
> correctly. Specifically, I believe you have correctly implemented what
> Andres described i.e. use CFI to get control to do the wrapping and
> then from the ExecProcNode wrapper do the actual EXPLAIN, whereas I
> believe Rafael was doing the wrapping directly from the signal
> handler, which did not seem safe to me:
> 
> http://postgr.es/m/CA+TgmobrzeDep+Z1BPQqGNsCqTQ8M58wNNKJB_8Lwpwbqbz3GQ@mail.gmail.com
> 
> I also like your version of (sub)transaction abort handling much
> better than the memory-context cleanup that Rafael initially chose.
> He's since revised that.
> 
> That said, I think the view that Rafael proposes, with a
> periodically-updating version of the EXPLAIN ANALYZE output up to that
> point, could be extremely useful in some situations.
+1.

> To make that
> work, he mostly just hacked InstrStopNode(), although as I'm thinking
> about it, that probably doesn't handle all of the parallel query cases
> correctly. I'm somewhat inclined to hope that we eventually end up
> with both interfaces.

>> I was considering whether to introduce a GUC in this patch to allow 
>> for
>> prior setup before outputting the plan or to switch to Rafael's patch
>> after reviewing its details. However, since there isn’t much time left
>> before the feature freeze, if you have already reviewed Rafael's patch
>> and there is a chance it could be committed, it would be better to 
>> focus
>> on that.
> 
> I wasn't feeling very confident about my ability to get that patch
> committed before feature freeze. I don't want to rush into something
> that we might later regret. I'm going to spend a bit more time
> studying your patch next.

That’s really appreciated!
I believe some of the comments in Rafael's thread should be reflected in 
this one as well, but I haven’t incorporated them yet. Apologies for 
that.


-- 
Regards,

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