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

Reply via email to