Re: Printing window function OVER clauses in EXPLAIN

On Mon, 10 Mar 2025 at 11:19, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> OK, here's v2 done like that. I do like this output better.
> I backed off the idea of putting the WindowClause as such
> into the plan, partly because I didn't feel like debugging
> the setrefs.c problem that David discovered upthread.
> This way does require a bit more code, but I think it's less
> likely to have bugs.

This output is much nicer. The patch looks good to me.

What are your thoughts on being a bit more brief with the naming and
just prefix with "w" instead of "window"? Looking at window.out, I see
that the EXPLAIN output does become quite a bit wider than before. I
favour the idea of saving a bit of space. There is an example in
src/sgml/advanced.sgml that has "OVER w", so it does not seem overly
strange to me to name them "w1", "w2", etc.

> * In passing, I editorialized on the order in which the Run Condition
> annotation comes out:
>
> case T_WindowAgg:
> + show_window_def(castNode(WindowAggState, planstate), ancestors, es);
> show_upper_qual(plan->qual, "Filter", planstate, ancestors, es);
> + show_upper_qual(((WindowAgg *) plan)->runConditionOrig,
> + "Run Condition", planstate, ancestors, es);
> if (plan->qual)
> show_instrumentation_count("Rows Removed by Filter", 1,
> planstate, es);
> - show_upper_qual(((WindowAgg *) plan)->runConditionOrig,
> - "Run Condition", planstate, ancestors, es);
> show_windowagg_info(castNode(WindowAggState, planstate), es);
> break;
>
> It seemed quite weird to me to have the Run Condition plan property
> come out in the middle of properties that only appear in EXPLAIN
> ANALYZE mode. Maybe there's a reason for this other than "people
> added new properties at the end", but I don't see it.

I did it that way because "Rows Removed by Filter" is a property of
"Filter", so it makes sense to me for those to be together. It doesn't
make sense to me to put something unrelated between them.

If you look at BitmapHeapScan output, this keeps the related outputs
together, i.e:

-> Parallel Bitmap Heap Scan on ab (cost=111.20..82787.64 rows=1
width=8) (actual time=172.498..172.499 rows=0.00 loops=3)
Recheck Cond: (a = 1)
Rows Removed by Index Recheck: 705225
Filter: (b = 3)
Rows Removed by Filter: 3333

What you're proposing seems equivalent to if we did it like:

-> Parallel Bitmap Heap Scan on ab (cost=111.20..82787.64 rows=1
width=8) (actual time=172.498..172.499 rows=0.00 loops=3)
Recheck Cond: (a = 1)
Filter: (b = 3)
Rows Removed by Index Recheck: 705225
Rows Removed by Filter: 3333

David