On Thu, Jun 16, 2016 at 4:31 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> However, before trying to fix any of that, I'd like to demand an
> explanation as to why aggoutputtype exists at all. It seems incredibly
> confusing to have both that and aggtype, and certainly the code comments
> give no hint of what the semantics are supposed to be when those fields
> are different. I think aggoutputtype needs to go away again.
fix_combine_agg_expr, or more search_indexed_tlist_for_partial_aggref,
needs to be able to match a finalize-aggregate node type to a
partial-aggregate node type beneath it. aggtype is one of the fields
that gets compared as part of that process, and it won't be the same
if you make aggtype mean the result of that particular node rather
than the result of the aggregate. nodeAgg.c also uses aggtype for
matching purposes; not sure if there is a similar problem there or
not.
On the other hand, exprType() needs to return the actual output type
of that particular node, which is given by aggoutputtype. I think
that requirement is non-negotiable, so I suspect the only way to get
by without two fields is if all the stuff for which aggtype is being
used is inessential; in that case, we could remove it and then rename
aggoutputtype back to aggtype.
Thanks for looking at this.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
