On 3/18/25 14:33, Robert Haas wrote:
> On Tue, Mar 18, 2025 at 8:02 AM Andrei Lepikhov <lepihov@gmail.com> wrote:
>> Some questions:
>> 1. I think, hooks ExplainOneQuery_hook_type, explain_per_plan_hook_type,
>> explain_per_node_hook_type deserve to be moved to explain_format.h
>> At least, inside the hook, we usually use functions like ExplainProperty.
>
> -1, because the hooks will be called from explain.c, not explain_state.c.
Ok.
>
>> 2. In my patch I inserted the hook before the line 1894:
>> /* in text format, the first line ends here */
>> Why have you chosen a different way? I don't have specific reasons to
>> insist, except the extension data right under the node looks better to
>> me personally.
>
> Tom discusses why we shouldn't try to add to the first line in
> http://postgr.es/m/2234935.1741809008@sss.pgh.pa.us and I'm fully in
> agreement.
I agree with him, too. But, as you can see, I proposed not changing the
first string or adding something there but just putting extension data
under that line. Extra information about workers' state (not so
important most of the time, I should say) sometimes makes it difficult
to read.
>
> I've committed 0001 and 0002 for now. The additional hook for
> cross-option validation can be added in a separate commit. v6-0003,
> now v7-0001, needs more substantive review before commit. I hope it
> gets some, and soon.
Ok, I am ready to review it thoroughly, if needed.
--
regards, Andrei Lepikhov
