Re: [PATCH] Add NESTED_STATEMENTS option to EXPLAIN

At 2026-06-02 11:01:26, "Mohamed ALi" <moali(dot)pg(at)gmail(dot)com> wrote:
>Attached is v5 of the patch.
>
>Regarding the trigger label scope question: the `(trigger)` annotation
>uses `GetMyTriggerDepth() > 0`, which labels ALL statements inside any
>trigger context (including cascading triggers). The nesting level
>already distinguishes direct vs cascade (level 2 = direct, level 3+ =
>cascade).
>
>Regarding statement ordering: v5 switches from completion order to
>start-time (chronological) order. The previous versions used completion
>order because it naturally matches how the executor hooks fire and is
>what auto_explain uses internally. However, I agree with your feedback
>that start-time order is better for users reading the output — parents
>appear before their children, which reads more naturally top-to-bottom.
>See the detailed section below.
>

>Changes since v4:
Hi,
I have some review feedback on the v5 patch.

diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 112c17b0d64..3392fceae34 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
...
+ else if (es->format == EXPLAIN_FORMAT_YAML)
+ {
+ /* Find "Execution Time:" line and append percentage after it */
+ char *et = strstr(info->plan_text, "Execution Time:");
+ if (et)
+ {
+ StringInfoData newbuf;
+ /* Find end of line */
+ char *eol = et;
+ while (*eol && *eol != '\n')
+ eol++;
+
+ initStringInfo(&newbuf);
+ appendBinaryStringInfo(&newbuf, info->plan_text,
+ eol - info->plan_text);
+ appendStringInfo(&newbuf,
+ "\n Execution Time Percentage: %.1f",
+ pct);
...

The patch works by first emitting the full JSON/XML/YAML plan string, then parsing,
slicing and stitching it via strstr, strchr and pointer arithmetic to inject additional fields,
alongside a few hardcoded special cases. I don’t find this implementation elegant.
When dealing with YAML output, this subtle design weakness can cause problems when locating where to insert new content in the generated YAML text.
For instance, consider a table named `public.Execution Time: T`.

CREATE TABLE "public.Execution Time: T" (id int);
INSERT INTO "public.Execution Time: T" VALUES (1);
CREATE FUNCTION f() RETURNS bigint AS $$
DECLARE r bigint;
BEGIN SELECT count(*) INTO r FROM "public.Execution Time: T"; RETURN r; END;
$$ LANGUAGE plpgsql;
EXPLAIN (FORMAT YAML, ANALYZE, NESTED_STATEMENTS, SUMMARY ON) SELECT f();

The line `Execution Time Percentage: 100.0` would then get inserted into the incorrect position.
I may be nitpicking, of course — table names of that form are virtually nonexistent in real-world usage.
Still, this approach feels somewhat hacky.

Best regards,

--

Yilin Zhang