I noticed a wrong comment in search_indexed_tlist_for_sortgroupref().
foreach(lc, itlist->tlist)
{
TargetEntry *tle = (TargetEntry *) lfirst(lc);
/* The equal() check should be redundant, but let's be paranoid */
if (tle->ressortgroupref == sortgroupref &&
equal(node, tle->expr))
{
It turns out that the equal() check is necessary, because the given
sort/group expression might be type of FuncExpr which converts integer
to numeric. In this case we need to modify its args not itself to
reference the matching subplan output expression. As an example,
consider
explain (costs off, verbose)
SELECT 1.1 AS two UNION (SELECT 2 UNION ALL SELECT 2);
QUERY PLAN
-------------------------------------
HashAggregate
Output: (1.1)
Group Key: (1.1)
-> Append
-> Result
Output: 1.1
-> Result
Output: (2)
-> Append
-> Result
Output: 2
-> Result
Output: 2
(13 rows)
If we remove the equal() check, this query would cause crash in
execution.
I'm considering changing the comment as below.
- /* The equal() check should be redundant, but let's be paranoid */
+ /*
+ * The equal() check is necessary, because expressions with the same
+ * sortgroupref might be different, e.g., the given sort/group
+ * expression can be type of FuncExpr which converts integer to
+ * numeric, and we need to modify its args not itself to reference the
+ * matching subplan output expression in this case.
+ */
Any thoughts?
Thanks
Richard
