Re: Bug in 9.0Alpha4

2010/3/17 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
> Gokulakannan Somasundaram <gokul007(at)gmail(dot)com> writes:
>>> transformSortClause is passed the untransformed aggorder list, which is
>>> in fact a list of SortBy nodes, and it returns the transformed list
>>> (SortGroupClause nodes), which is stored back into the aggorder field
>>> a bit further down.
>
>> Right Tom.  I got confused, because the comment at Aggref struct definition
>> told that it is a list of SortGroupClause. May be you can update your
>> comments there.
>
> I think that comment is fine.  The reason this is confusing is that
> ParseFuncOrColumn uses the Aggref node to carry a couple of things
> that logically are input parameters to transformAggregateCall().
> Although this affects nothing else and is commented at both ends,
> apparently it's confusing anyway.
>
> When we were doing the ordered-aggregates patch, I considered passing
> all those values as explicit parameters to transformAggregateCall,
> and having it build the Aggref node from scratch and return it.
> However having seven or eight parameters to transformAggregateCall
> (and more in future if we ever add more features here) didn't really
> seem to be better style than abusing Aggref a bit.  But maybe it is
> the best way after all.  Thoughts?

Well, I think the point is args and aggorder are hidden in the Aggref
passed to transformAggregateCall, although they will be transformed in
the function. Isn't it enough to add more parameters for them
(agg_distinct is passed separately) and to leave the Aggref pointer
passing as present?

Regards,

--
Hitoshi Harada