Re: Convert planner's AggInfo and AggTransInfo to Nodes

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:

> I got annoyed just now upon finding that pprint() applied to the planner's
> "root" pointer doesn't dump root->agginfos or root->aggtransinfos. That's
> evidently because AggInfo and AggTransInfo aren't proper Nodes, just bare
> structs, which presumably is because somebody couldn't be bothered to
> write outfuncs support for them. I'd say that was a questionable shortcut
> even when it was made, and there's certainly precious little excuse now
> that gen_node_support.pl can do all the heavy lifting. Hence, PFA a
> little finger exercise to turn them into Nodes. I took the opportunity
> to improve related comments too, and in particular to fix some comments
> that leave the impression that preprocess_minmax_aggregates still does
> its own scan of the query tree. (It was momentary confusion over that
> idea that got me to the point of being annoyed in the first place.)
>
> Any objections so far?

It seems like a reasonable idea, but I don't know enough to judge the
wider ramifications of it. But one thing that the patch should also do,
is switch to using the l*_node() functions instead of manual casting.

The ones I noticed in the patch/context are below, but there are a few
more:

> foreach(lc, root->agginfos)
> {
> AggInfo *agginfo = (AggInfo *) lfirst(lc);

AggInfo *agginfo = lfirst_node(AggInfo, lc);

[…]
> foreach(lc, transnos)
> {
> int transno = lfirst_int(lc);
> - AggTransInfo *pertrans = (AggTransInfo *) list_nth(root->aggtransinfos, transno);
> + AggTransInfo *pertrans = (AggTransInfo *) list_nth(root->aggtransinfos,
> + transno);
> + AggTransInfo *pertrans = (AggTransInfo *) list_nth(root->aggtransinfos,
> + transno);

AggTransInfo *pertrans = list_nth_node(AggTransInfo, root->aggtransinfos,
transno);

- ilmari