On 2025-04-01 21:32, Robert Haas wrote:
Thanks for your comment.
On Mon, Mar 31, 2025 at 9:43 PM torikoshia <torikos...@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_8lwpwbqbz...@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.
