Message-ID: <4be5223e-4725-49a2-b49f-769f4f402984@gmail.com>
Date: Fri, 28 Mar 2025 16:09:39 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Proposal: Progressive explain
To: Rafael Thofehrn Castro <rafaelthca@gmail.com>,
 Robert Haas <robertmhaas@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
References: 
 <CAG0ozMpD27fCyVFoa82G7uV4md4wVYw7DWC-PVA6P5=1v35tsA@mail.gmail.com>
 <CAG0ozMqfQjO=xK-zwfvNGvpZnWtynB6iXxCShcH5t5vsYV7_LA@mail.gmail.com>
 <CAG0ozMoaV_C1_Sf7QMjfCjUODL8U959gvYWPrVSjgvc2gcRyxQ@mail.gmail.com>
 <CAG0ozMo30smtXXOR8bSCbhaZAQHo4=ezerLitpERk85Q0ga+Fw@mail.gmail.com>
 <CAG0ozMo4eaJJqqiHP7dHXm=u8vK-qqxo5Q1DVu=amHisZiX5_g@mail.gmail.com>
 <d61f6093-5210-42c3-9f82-33f21ac6e2db@app.fastmail.com>
 <CAG0ozMpwRQFNraWLGVa0zAFBYuY1HasZ6NX4BAM4H3OOrQHtDg@mail.gmail.com>
 <CAG0ozMrZ+dG9TA+X54Tjwxm76O2_u97_Yvhdf3ixx+0caGF4Mg@mail.gmail.com>
 <CA+TgmoaD985+VLwR93c8PjSaoBqxw72Eu7pfBJcArzhjJ71aRw@mail.gmail.com>
 <CAG0ozMqcsZFmaeodUA9tYbbZsYJFYgJVYxTpa=ggee+ShvWQCA@mail.gmail.com>
 <CA+TgmoZw+xy3WyrPD4X5ChP+=kJ4ncRZY8xQu0XfChDa4fa7yg@mail.gmail.com>
 <CAG0ozMpjEFThDGNqSoFTj1v_cpEs0rRQw5moVKU6Oy88oQ=Yhw@mail.gmail.com>
 <CA+TgmoZ51Hc7o_vFo8rbPqkLpSVzH6i=ccPrt2mHBPyk=zMxwQ@mail.gmail.com>
 <CAG0ozMq48+ODFoLW4HHexN8BKHfh_4c-n2b=yH9oaUnCZNZj6Q@mail.gmail.com>
Content-Language: en-AU
From: Andrei Lepikhov <lepihov@gmail.com>
In-Reply-To: 
 <CAG0ozMq48+ODFoLW4HHexN8BKHfh_4c-n2b=yH9oaUnCZNZj6Q@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/4be5223e-4725-49a2-b49f-769f4f402984%40gmail.com>
Precedence: bulk

On 3/26/25 22:57, Rafael Thofehrn Castro wrote:
> So implementation was done based on transaction nested level. Cleanup is 
> only
> performed when AbortSubTransaction() is called in the same transaction 
> nested
> level as the one where the query is running. This covers both PL/pgSQL 
> exception
> blocks and savepoints.
Thanks for your efforts!
Let me provide an alternative view of your code.
Postgres has a massive gap between the start of the execution and the 
end. All the stuff that happens at that time can't be discovered. That's 
sad. If you add a new hook into the ExecuteNode (it may be designed as a 
pointer in the PlanState to affect only necessary nodes), you may push 
the code out of the core and give other extensions additional tools.
I see your reasoning here [1], but I think with the commit 4fd02bf7cf9, 
it should be revised.
As I see after reviewing your code, to design it as an extension, some 
parallel query execution stuff needs to be exported, and Instrumentation 
needs to be changed a little. For example, with progressive explain we 
have a new node state - 'not visited yet' that is different from 'not 
executed'.
What is the purpose of a new hook from a broad perspective? I designed 
at least two features with such a hook in the Postgres fork: 1) 
real-time execution statistics and 2) query interruption on the 
arbitrary trigger (planning error detected, a signal arrived, too much 
temp memory allocated, etc.). I am not sure if it would be interesting 
for the community, but when a query is executed for more than one 
minute, I certainly want to control what is happening and have some 
tools except query abortion.
The benefit of such an approach is that it is doable to change the 
Instrumentation, add a hook now, and develop this extension without a 
rush until it is stable - I think at least the case of parallel 
execution may be enhanced.

[1] 
https://www.postgresql.org/message-id/CAG0ozMrtK_u8Uf5KNZUmRNuMphV5tnC5DEhRBNRGK%2BK4L506xw%40mail.gmail.com

-- 
regards, Andrei Lepikhov