Bug in 9.0Alpha4
in [PgSql]
|
From: Hitoshi Harada on 16 Mar 2010 21:17 2010/3/17 Tom Lane <tgl(a)sss.pgh.pa.us>: > Gokulakannan Somasundaram <gokul007(a)gmail.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Gokulakannan Somasundaram on 17 Mar 2010 04:30 > > > 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? > > I feel it would be good, if we send the parameters explicitly and if that increases, put it inside another structure(data carriage structure) and send it.. But please take my suggestion as a novice one. :)) Thanks, Gokul.
From: Tom Lane on 17 Mar 2010 12:00
Hitoshi Harada <umi.tanuki(a)gmail.com> writes: > 2010/3/17 Tom Lane <tgl(a)sss.pgh.pa.us>: >> 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? Yeah, that's probably the least complicated solution. Will fix. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers |