On Sat, 2006-03-04 at 21:21 -0500, Tom Lane wrote:
> 1. For each ORDER BY item:
> 1a. Find a matching GROUP BY item.
> (Break out of loop if no match.)
> 1b. Add it to the output list with ORDER BY item's ordering op.
> 1c. Remove the matched item from the GROUP BY list.
> 2. For each remaining GROUP BY item:
> 2a. Add it to output list using default ordering op.
Okay, attached is a revised patch that implements this. Barring any
objections I'll apply it tomorrow.
-Neil
============================================================
*** src/backend/parser/parse_clause.c 1b13820b23dd57bd4e00ffbd2f979015b1536243
--- src/backend/parser/parse_clause.c e7f9499effe2dbe572668c83ed455b49299954af
***************
*** 1296,1333 ****
return target_result;
}
/*
* transformGroupClause -
* transform a GROUP BY clause
*
* GROUP BY items will be added to the targetlist (as resjunk columns)
* if not already present, so the targetlist must be passed by reference.
*/
List *
transformGroupClause(ParseState *pstate, List *grouplist,
List **targetlist, List *sortClause)
{
! List *glist = NIL;
! ListCell *gl;
! ListCell *sortItem;
! sortItem = list_head(sortClause);
!
! foreach(gl, grouplist)
{
TargetEntry *tle;
! Oid restype;
! Oid ordering_op;
! GroupClause *grpcl;
! tle = findTargetlistEntry(pstate, lfirst(gl),
targetlist, GROUP_CLAUSE);
- /* avoid making duplicate grouplist entries */
- if (targetIsInSortList(tle, glist))
- continue;
-
/* if tlist item is an UNKNOWN literal, change it to TEXT */
restype = exprType((Node *) tle->expr);
--- 1296,1341 ----
return target_result;
}
+ static GroupClause *
+ make_group_clause(TargetEntry *tle, List *targetlist, Oid sortop)
+ {
+ GroupClause *result;
+ result = makeNode(GroupClause);
+ result->tleSortGroupRef = assignSortGroupRef(tle, targetlist);
+ result->sortop = sortop;
+ return result;
+ }
+
/*
* transformGroupClause -
* transform a GROUP BY clause
*
* GROUP BY items will be added to the targetlist (as resjunk columns)
* if not already present, so the targetlist must be passed by reference.
+ *
+ * The order of the elements of the grouping clause does not affect
+ * the semantics of the query. However, the optimizer is not currently
+ * smart enough to reorder the grouping clause, so we try to do some
+ * primitive reordering here.
*/
List *
transformGroupClause(ParseState *pstate, List *grouplist,
List **targetlist, List *sortClause)
{
! List *result = NIL;
! List *tle_list = NIL;
! ListCell *l;
! /* Preprocess the grouping clause, lookup TLEs */
! foreach (l, grouplist)
{
TargetEntry *tle;
! Oid restype;
! tle = findTargetlistEntry(pstate, lfirst(l),
targetlist, GROUP_CLAUSE);
/* if tlist item is an UNKNOWN literal, change it to TEXT */
restype = exprType((Node *) tle->expr);
***************
*** 1332,1356 ****
restype = exprType((Node *) tle->expr);
if (restype == UNKNOWNOID)
- {
tle->expr = (Expr *) coerce_type(pstate, (Node *) tle->expr,
restype, TEXTOID, -1,
COERCION_IMPLICIT,
COERCE_IMPLICIT_CAST);
- restype = TEXTOID;
- }
! /*
! * If the GROUP BY clause matches the ORDER BY clause, we want to
! * adopt the ordering operators from the latter rather than using the
! * default ops. This allows "GROUP BY foo ORDER BY foo DESC" to be
! * done with only one sort step. Note we are assuming that any
! * user-supplied ordering operator will bring equal values together,
! * which is all that GROUP BY needs.
! */
! if (sortItem &&
! ((SortClause *) lfirst(sortItem))->tleSortGroupRef ==
! tle->ressortgroupref)
{
ordering_op = ((SortClause *) lfirst(sortItem))->sortop;
sortItem = lnext(sortItem);
--- 1340,1369 ----
restype = exprType((Node *) tle->expr);
if (restype == UNKNOWNOID)
tle->expr = (Expr *) coerce_type(pstate, (Node *) tle->expr,
restype, TEXTOID, -1,
COERCION_IMPLICIT,
COERCE_IMPLICIT_CAST);
! tle_list = lappend(tle_list, tle);
! }
!
! /*
! * Now iterate through the ORDER BY clause. If we find a grouping
! * element that matches the ORDER BY element, append the grouping
! * element to the result set immediately. Otherwise, stop
! * iterating. The effect of this is to look for a prefix of the
! * ORDER BY list in the grouping clauses, and to move that prefix
! * to the front of the GROUP BY.
! */
! foreach (l, sortClause)
! {
! SortClause *sc = (SortClause *) lfirst(l);
! ListCell *prev = NULL;
! ListCell *tl;
! bool found = false;
!
! foreach (tl, tle_list)
{
TargetEntry *tle = (TargetEntry *) lfirst(tl);
***************
*** 1352,1373 ****
((SortClause *) lfirst(sortItem))->tleSortGroupRef ==
tle->ressortgroupref)
{
! ordering_op = ((SortClause *) lfirst(sortItem))->sortop;
! sortItem = lnext(sortItem);
}
- else
- {
- ordering_op = ordering_oper_opid(restype);
- sortItem = NULL; /* disregard ORDER BY once match fails */
- }
! grpcl = makeNode(GroupClause);
! grpcl->tleSortGroupRef = assignSortGroupRef(tle, *targetlist);
! grpcl->sortop = ordering_op;
! glist = lappend(glist, grpcl);
}
! return glist;
}
/*
--- 1365,1420 ----
((SortClause *) lfirst(sortItem))->tleSortGroupRef ==
tle->ressortgroupref)
{
! TargetEntry *tle = (TargetEntry *) lfirst(tl);
!
! if (sc->tleSortGroupRef == tle->ressortgroupref)
! {
! GroupClause *gc;
!
! tle_list = list_delete_cell(tle_list, tl, prev);
!
! /*
! * This grouping clause matches one of the sorting
! * clause's ordering operator.
! */
! gc = make_group_clause(tle, *targetlist, sc->sortop);
! result = lappend(result, gc);
! found = true;
! break;
! }
!
! prev = tl;
}
! /* As soon as we've failed to match an ORDER BY element, stop */
! if (!found)
! break;
}
! /*
! * Now add any remaining elements of the GROUP BY list in the
! * order we received them.
! *
! * XXX: are there any additional criteria to consider when
! * ordering grouping clauses?
! */
! foreach(l, tle_list)
! {
! TargetEntry *tle = (TargetEntry *) lfirst(l);
! GroupClause *gc;
! Oid sort_op;
!
! /* avoid making duplicate grouplist entries */
! if (targetIsInSortList(tle, result))
! continue;
!
! sort_op = ordering_oper_opid(exprType((Node *) tle->expr));
! gc = make_group_clause(tle, *targetlist, sort_op);
! result = lappend(result, gc);
! }
!
! list_free(tle_list);
! return result;
}
/*
---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to [EMAIL PROTECTED] so that your
message can get through to the mailing list cleanly