window-func-fixes-v2.patch

diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c
index 968d5d3..0db193b 100644
*** a/src/backend/executor/nodeWindowAgg.c
--- b/src/backend/executor/nodeWindowAgg.c
*************** begin_partition(WindowAggState *winstate
*** 1079,1084 ****
--- 1079,1085 ----
  {
  	WindowAgg  *node = (WindowAgg *) winstate->ss.ps.plan;
  	PlanState  *outerPlan = outerPlanState(winstate);
+ 	int			frameOptions = winstate->frameOptions;
  	int			numfuncs = winstate->numfuncs;
  	int			i;
  
*************** begin_partition(WindowAggState *winstate
*** 1143,1150 ****
  		 * If the frame head is potentially movable, or we have an EXCLUSION
  		 * clause, we might need to restart aggregation ...
  		 */
! 		if (!(winstate->frameOptions & FRAMEOPTION_START_UNBOUNDED_PRECEDING) ||
! 			(winstate->frameOptions & FRAMEOPTION_EXCLUSION))
  		{
  			/* ... so create a mark pointer to track the frame head */
  			agg_winobj->markptr = tuplestore_alloc_read_pointer(winstate->buffer, 0);
--- 1144,1151 ----
  		 * If the frame head is potentially movable, or we have an EXCLUSION
  		 * clause, we might need to restart aggregation ...
  		 */
! 		if (!(frameOptions & FRAMEOPTION_START_UNBOUNDED_PRECEDING) ||
! 			(frameOptions & FRAMEOPTION_EXCLUSION))
  		{
  			/* ... so create a mark pointer to track the frame head */
  			agg_winobj->markptr = tuplestore_alloc_read_pointer(winstate->buffer, 0);
*************** begin_partition(WindowAggState *winstate
*** 1182,1202 ****
  
  	/*
  	 * If we are in RANGE or GROUPS mode, then determining frame boundaries
! 	 * requires physical access to the frame endpoint rows, except in
  	 * degenerate cases.  We create read pointers to point to those rows, to
  	 * simplify access and ensure that the tuplestore doesn't discard the
! 	 * endpoint rows prematurely.  (Must match logic in update_frameheadpos
! 	 * and update_frametailpos.)
  	 */
  	winstate->framehead_ptr = winstate->frametail_ptr = -1; /* if not used */
  
! 	if ((winstate->frameOptions & (FRAMEOPTION_RANGE | FRAMEOPTION_GROUPS)) &&
! 		node->ordNumCols != 0)
  	{
! 		if (!(winstate->frameOptions & FRAMEOPTION_START_UNBOUNDED_PRECEDING))
  			winstate->framehead_ptr =
  				tuplestore_alloc_read_pointer(winstate->buffer, 0);
! 		if (!(winstate->frameOptions & FRAMEOPTION_END_UNBOUNDED_FOLLOWING))
  			winstate->frametail_ptr =
  				tuplestore_alloc_read_pointer(winstate->buffer, 0);
  	}
--- 1183,1206 ----
  
  	/*
  	 * If we are in RANGE or GROUPS mode, then determining frame boundaries
! 	 * requires physical access to the frame endpoint rows, except in certain
  	 * degenerate cases.  We create read pointers to point to those rows, to
  	 * simplify access and ensure that the tuplestore doesn't discard the
! 	 * endpoint rows prematurely.  (Must create pointers in exactly the same
! 	 * cases that update_frameheadpos and update_frametailpos need them.)
  	 */
  	winstate->framehead_ptr = winstate->frametail_ptr = -1; /* if not used */
  
! 	if (frameOptions & (FRAMEOPTION_RANGE | FRAMEOPTION_GROUPS))
  	{
! 		if (((frameOptions & FRAMEOPTION_START_CURRENT_ROW) &&
! 			 node->ordNumCols != 0) ||
! 			(frameOptions & FRAMEOPTION_START_OFFSET))
  			winstate->framehead_ptr =
  				tuplestore_alloc_read_pointer(winstate->buffer, 0);
! 		if (((frameOptions & FRAMEOPTION_END_CURRENT_ROW) &&
! 			 node->ordNumCols != 0) ||
! 			(frameOptions & FRAMEOPTION_END_OFFSET))
  			winstate->frametail_ptr =
  				tuplestore_alloc_read_pointer(winstate->buffer, 0);
  	}
*************** begin_partition(WindowAggState *winstate
*** 1210,1217 ****
  	 */
  	winstate->grouptail_ptr = -1;
  
! 	if ((winstate->frameOptions & (FRAMEOPTION_EXCLUDE_GROUP |
! 								   FRAMEOPTION_EXCLUDE_TIES)) &&
  		node->ordNumCols != 0)
  	{
  		winstate->grouptail_ptr =
--- 1214,1221 ----
  	 */
  	winstate->grouptail_ptr = -1;
  
! 	if ((frameOptions & (FRAMEOPTION_EXCLUDE_GROUP |
! 						 FRAMEOPTION_EXCLUDE_TIES)) &&
  		node->ordNumCols != 0)
  	{
  		winstate->grouptail_ptr =
*************** update_frameheadpos(WindowAggState *wins
*** 1563,1568 ****
--- 1567,1575 ----
  			bool		sub,
  						less;
  
+ 			/* We must have an ordering column */
+ 			Assert(node->ordNumCols == 1);
+ 
  			/* Precompute flags for in_range checks */
  			if (frameOptions & FRAMEOPTION_START_OFFSET_PRECEDING)
  				sub = true;		/* subtract startOffset from current row */
*************** update_frameheadpos(WindowAggState *wins
*** 1643,1648 ****
--- 1650,1660 ----
  			 * framehead_slot, and advance as necessary.  Note that if we
  			 * reach end of partition, we will leave frameheadpos = end+1 and
  			 * framehead_slot empty.
+ 			 *
+ 			 * Note: it's possible to have ordNumCols == 0, in which case
+ 			 * there is only one peer group and we could in principle do this
+ 			 * without the laborious per-row tests.  Doesn't seem worth
+ 			 * expending extra code on such a silly case, though.
  			 */
  			int64		offset = DatumGetInt64(winstate->startOffsetValue);
  			int64		minheadgroup;
*************** update_frametailpos(WindowAggState *wins
*** 1814,1819 ****
--- 1826,1834 ----
  			bool		sub,
  						less;
  
+ 			/* We must have an ordering column */
+ 			Assert(node->ordNumCols == 1);
+ 
  			/* Precompute flags for in_range checks */
  			if (frameOptions & FRAMEOPTION_END_OFFSET_PRECEDING)
  				sub = true;		/* subtract endOffset from current row */
*************** update_frametailpos(WindowAggState *wins
*** 1894,1899 ****
--- 1909,1919 ----
  			 * of the last-known frame tail row in frametail_slot, and advance
  			 * as necessary.  Note that if we reach end of partition, we will
  			 * leave frametailpos = end+1 and frametail_slot empty.
+ 			 *
+ 			 * Note: it's possible to have ordNumCols == 0, in which case
+ 			 * there is only one peer group and we could in principle do this
+ 			 * without the laborious per-row tests.  Doesn't seem worth
+ 			 * expending extra code on such a silly case, though.
  			 */
  			int64		offset = DatumGetInt64(winstate->endOffsetValue);
  			int64		maxtailgroup;
*************** ExecInitWindowAgg(WindowAgg *node, EStat
*** 2318,2333 ****
  	winstate->temp_slot_2 = ExecInitExtraTupleSlot(estate, scanDesc);
  
  	/*
! 	 * create frame head and tail slots only if needed (must match logic in
! 	 * update_frameheadpos and update_frametailpos)
  	 */
  	winstate->framehead_slot = winstate->frametail_slot = NULL;
  
  	if (frameOptions & (FRAMEOPTION_RANGE | FRAMEOPTION_GROUPS))
  	{
! 		if (!(frameOptions & FRAMEOPTION_START_UNBOUNDED_PRECEDING))
  			winstate->framehead_slot = ExecInitExtraTupleSlot(estate, scanDesc);
! 		if (!(frameOptions & FRAMEOPTION_END_UNBOUNDED_FOLLOWING))
  			winstate->frametail_slot = ExecInitExtraTupleSlot(estate, scanDesc);
  	}
  
--- 2338,2358 ----
  	winstate->temp_slot_2 = ExecInitExtraTupleSlot(estate, scanDesc);
  
  	/*
! 	 * create frame head and tail slots only if needed (must create slots in
! 	 * exactly the same cases that update_frameheadpos and update_frametailpos
! 	 * need them)
  	 */
  	winstate->framehead_slot = winstate->frametail_slot = NULL;
  
  	if (frameOptions & (FRAMEOPTION_RANGE | FRAMEOPTION_GROUPS))
  	{
! 		if (((frameOptions & FRAMEOPTION_START_CURRENT_ROW) &&
! 			 node->ordNumCols != 0) ||
! 			(frameOptions & FRAMEOPTION_START_OFFSET))
  			winstate->framehead_slot = ExecInitExtraTupleSlot(estate, scanDesc);
! 		if (((frameOptions & FRAMEOPTION_END_CURRENT_ROW) &&
! 			 node->ordNumCols != 0) ||
! 			(frameOptions & FRAMEOPTION_END_OFFSET))
  			winstate->frametail_slot = ExecInitExtraTupleSlot(estate, scanDesc);
  	}
  
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index cf82b70..4186e20 100644
*** a/src/backend/optimizer/plan/createplan.c
--- b/src/backend/optimizer/plan/createplan.c
*************** static WindowAgg *create_windowagg_plan(
*** 107,121 ****
  static SetOp *create_setop_plan(PlannerInfo *root, SetOpPath *best_path,
  				  int flags);
  static RecursiveUnion *create_recursiveunion_plan(PlannerInfo *root, RecursiveUnionPath *best_path);
- static void get_column_info_for_window(PlannerInfo *root, WindowClause *wc,
- 						   List *tlist,
- 						   int numSortCols, AttrNumber *sortColIdx,
- 						   int *partNumCols,
- 						   AttrNumber **partColIdx,
- 						   Oid **partOperators,
- 						   int *ordNumCols,
- 						   AttrNumber **ordColIdx,
- 						   Oid **ordOperators);
  static LockRows *create_lockrows_plan(PlannerInfo *root, LockRowsPath *best_path,
  					 int flags);
  static ModifyTable *create_modifytable_plan(PlannerInfo *root, ModifyTablePath *best_path);
--- 107,112 ----
*************** create_windowagg_plan(PlannerInfo *root,
*** 2160,2178 ****
  {
  	WindowAgg  *plan;
  	WindowClause *wc = best_path->winclause;
  	Plan	   *subplan;
  	List	   *tlist;
- 	int			numsortkeys;
- 	AttrNumber *sortColIdx;
- 	Oid		   *sortOperators;
- 	Oid		   *collations;
- 	bool	   *nullsFirst;
  	int			partNumCols;
  	AttrNumber *partColIdx;
  	Oid		   *partOperators;
  	int			ordNumCols;
  	AttrNumber *ordColIdx;
  	Oid		   *ordOperators;
  
  	/*
  	 * WindowAgg can project, so no need to be terribly picky about child
--- 2151,2167 ----
  {
  	WindowAgg  *plan;
  	WindowClause *wc = best_path->winclause;
+ 	int			numPart = list_length(wc->partitionClause);
+ 	int			numOrder = list_length(wc->orderClause);
  	Plan	   *subplan;
  	List	   *tlist;
  	int			partNumCols;
  	AttrNumber *partColIdx;
  	Oid		   *partOperators;
  	int			ordNumCols;
  	AttrNumber *ordColIdx;
  	Oid		   *ordOperators;
+ 	ListCell   *lc;
  
  	/*
  	 * WindowAgg can project, so no need to be terribly picky about child
*************** create_windowagg_plan(PlannerInfo *root,
*** 2183,2214 ****
  	tlist = build_path_tlist(root, &best_path->path);
  
  	/*
! 	 * We shouldn't need to actually sort, but it's convenient to use
! 	 * prepare_sort_from_pathkeys to identify the input's sort columns.
  	 */
! 	subplan = prepare_sort_from_pathkeys(subplan,
! 										 best_path->winpathkeys,
! 										 NULL,
! 										 NULL,
! 										 false,
! 										 &numsortkeys,
! 										 &sortColIdx,
! 										 &sortOperators,
! 										 &collations,
! 										 &nullsFirst);
  
! 	/* Now deconstruct that into partition and ordering portions */
! 	get_column_info_for_window(root,
! 							   wc,
! 							   subplan->targetlist,
! 							   numsortkeys,
! 							   sortColIdx,
! 							   &partNumCols,
! 							   &partColIdx,
! 							   &partOperators,
! 							   &ordNumCols,
! 							   &ordColIdx,
! 							   &ordOperators);
  
  	/* And finally we can make the WindowAgg node */
  	plan = make_windowagg(tlist,
--- 2172,2214 ----
  	tlist = build_path_tlist(root, &best_path->path);
  
  	/*
! 	 * Convert SortGroupClause lists into arrays of attr indexes and equality
! 	 * operators, as wanted by executor.  (Note: in principle, it's possible
! 	 * to drop some of the sort columns, if they were proved redundant by
! 	 * pathkey logic.  However, it doesn't seem worth going out of our way to
! 	 * optimize such cases.  In any case, we must *not* remove the ordering
! 	 * column for RANGE OFFSET cases, as the executor needs that for in_range
! 	 * tests even if it's known to be equal to some partitioning column.)
  	 */
! 	partColIdx = (AttrNumber *) palloc(sizeof(AttrNumber) * numPart);
! 	partOperators = (Oid *) palloc(sizeof(Oid) * numPart);
  
! 	partNumCols = 0;
! 	foreach(lc, wc->partitionClause)
! 	{
! 		SortGroupClause *sgc = (SortGroupClause *) lfirst(lc);
! 		TargetEntry *tle = get_sortgroupclause_tle(sgc, subplan->targetlist);
! 
! 		Assert(OidIsValid(sgc->eqop));
! 		partColIdx[partNumCols] = tle->resno;
! 		partOperators[partNumCols] = sgc->eqop;
! 		partNumCols++;
! 	}
! 
! 	ordColIdx = (AttrNumber *) palloc(sizeof(AttrNumber) * numOrder);
! 	ordOperators = (Oid *) palloc(sizeof(Oid) * numOrder);
! 
! 	ordNumCols = 0;
! 	foreach(lc, wc->orderClause)
! 	{
! 		SortGroupClause *sgc = (SortGroupClause *) lfirst(lc);
! 		TargetEntry *tle = get_sortgroupclause_tle(sgc, subplan->targetlist);
! 
! 		Assert(OidIsValid(sgc->eqop));
! 		ordColIdx[ordNumCols] = tle->resno;
! 		ordOperators[ordNumCols] = sgc->eqop;
! 		ordNumCols++;
! 	}
  
  	/* And finally we can make the WindowAgg node */
  	plan = make_windowagg(tlist,
*************** create_windowagg_plan(PlannerInfo *root,
*** 2235,2346 ****
  }
  
  /*
-  * get_column_info_for_window
-  *		Get the partitioning/ordering column numbers and equality operators
-  *		for a WindowAgg node.
-  *
-  * This depends on the behavior of planner.c's make_pathkeys_for_window!
-  *
-  * We are given the target WindowClause and an array of the input column
-  * numbers associated with the resulting pathkeys.  In the easy case, there
-  * are the same number of pathkey columns as partitioning + ordering columns
-  * and we just have to copy some data around.  However, it's possible that
-  * some of the original partitioning + ordering columns were eliminated as
-  * redundant during the transformation to pathkeys.  (This can happen even
-  * though the parser gets rid of obvious duplicates.  A typical scenario is a
-  * window specification "PARTITION BY x ORDER BY y" coupled with a clause
-  * "WHERE x = y" that causes the two sort columns to be recognized as
-  * redundant.)	In that unusual case, we have to work a lot harder to
-  * determine which keys are significant.
-  *
-  * The method used here is a bit brute-force: add the sort columns to a list
-  * one at a time and note when the resulting pathkey list gets longer.  But
-  * it's a sufficiently uncommon case that a faster way doesn't seem worth
-  * the amount of code refactoring that'd be needed.
-  */
- static void
- get_column_info_for_window(PlannerInfo *root, WindowClause *wc, List *tlist,
- 						   int numSortCols, AttrNumber *sortColIdx,
- 						   int *partNumCols,
- 						   AttrNumber **partColIdx,
- 						   Oid **partOperators,
- 						   int *ordNumCols,
- 						   AttrNumber **ordColIdx,
- 						   Oid **ordOperators)
- {
- 	int			numPart = list_length(wc->partitionClause);
- 	int			numOrder = list_length(wc->orderClause);
- 
- 	if (numSortCols == numPart + numOrder)
- 	{
- 		/* easy case */
- 		*partNumCols = numPart;
- 		*partColIdx = sortColIdx;
- 		*partOperators = extract_grouping_ops(wc->partitionClause);
- 		*ordNumCols = numOrder;
- 		*ordColIdx = sortColIdx + numPart;
- 		*ordOperators = extract_grouping_ops(wc->orderClause);
- 	}
- 	else
- 	{
- 		List	   *sortclauses;
- 		List	   *pathkeys;
- 		int			scidx;
- 		ListCell   *lc;
- 
- 		/* first, allocate what's certainly enough space for the arrays */
- 		*partNumCols = 0;
- 		*partColIdx = (AttrNumber *) palloc(numPart * sizeof(AttrNumber));
- 		*partOperators = (Oid *) palloc(numPart * sizeof(Oid));
- 		*ordNumCols = 0;
- 		*ordColIdx = (AttrNumber *) palloc(numOrder * sizeof(AttrNumber));
- 		*ordOperators = (Oid *) palloc(numOrder * sizeof(Oid));
- 		sortclauses = NIL;
- 		pathkeys = NIL;
- 		scidx = 0;
- 		foreach(lc, wc->partitionClause)
- 		{
- 			SortGroupClause *sgc = (SortGroupClause *) lfirst(lc);
- 			List	   *new_pathkeys;
- 
- 			sortclauses = lappend(sortclauses, sgc);
- 			new_pathkeys = make_pathkeys_for_sortclauses(root,
- 														 sortclauses,
- 														 tlist);
- 			if (list_length(new_pathkeys) > list_length(pathkeys))
- 			{
- 				/* this sort clause is actually significant */
- 				(*partColIdx)[*partNumCols] = sortColIdx[scidx++];
- 				(*partOperators)[*partNumCols] = sgc->eqop;
- 				(*partNumCols)++;
- 				pathkeys = new_pathkeys;
- 			}
- 		}
- 		foreach(lc, wc->orderClause)
- 		{
- 			SortGroupClause *sgc = (SortGroupClause *) lfirst(lc);
- 			List	   *new_pathkeys;
- 
- 			sortclauses = lappend(sortclauses, sgc);
- 			new_pathkeys = make_pathkeys_for_sortclauses(root,
- 														 sortclauses,
- 														 tlist);
- 			if (list_length(new_pathkeys) > list_length(pathkeys))
- 			{
- 				/* this sort clause is actually significant */
- 				(*ordColIdx)[*ordNumCols] = sortColIdx[scidx++];
- 				(*ordOperators)[*ordNumCols] = sgc->eqop;
- 				(*ordNumCols)++;
- 				pathkeys = new_pathkeys;
- 			}
- 		}
- 		/* complain if we didn't eat exactly the right number of sort cols */
- 		if (scidx != numSortCols)
- 			elog(ERROR, "failed to deconstruct sort operators into partitioning/ordering operators");
- 	}
- }
- 
- /*
   * create_setop_plan
   *
   *	  Create a SetOp plan for 'best_path' and (recursively) plans
--- 2235,2240 ----
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index fd45c97..2c31b57 100644
*** a/src/backend/optimizer/plan/planner.c
--- b/src/backend/optimizer/plan/planner.c
*************** make_window_input_target(PlannerInfo *ro
*** 5466,5473 ****
   * The required ordering is first the PARTITION keys, then the ORDER keys.
   * In the future we might try to implement windowing using hashing, in which
   * case the ordering could be relaxed, but for now we always sort.
-  *
-  * Caution: if you change this, see createplan.c's get_column_info_for_window!
   */
  static List *
  make_pathkeys_for_window(PlannerInfo *root, WindowClause *wc,
--- 5466,5471 ----
diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index e147880..cfd4b91 100644
*** a/src/backend/parser/parse_clause.c
--- b/src/backend/parser/parse_clause.c
*************** transformWindowDefinitions(ParseState *p
*** 2795,2800 ****
--- 2795,2810 ----
  			wc->inRangeNullsFirst = sortcl->nulls_first;
  		}
  
+ 		/* Per spec, GROUPS mode requires an ORDER BY clause */
+ 		if (wc->frameOptions & FRAMEOPTION_GROUPS)
+ 		{
+ 			if (wc->orderClause == NIL)
+ 				ereport(ERROR,
+ 						(errcode(ERRCODE_WINDOWING_ERROR),
+ 						 errmsg("GROUPS mode requires an ORDER BY clause"),
+ 						 parser_errposition(pstate, windef->location)));
+ 		}
+ 
  		/* Process frame offset expressions */
  		wc->startOffset = transformFrameOffset(pstate, wc->frameOptions,
  											   rangeopfamily, rangeopcintype,
diff --git a/src/test/regress/expected/window.out b/src/test/regress/expected/window.out
index 85d81e7..562006a 100644
*** a/src/test/regress/expected/window.out
--- b/src/test/regress/expected/window.out
*************** SELECT count(*) OVER (PARTITION BY four)
*** 2834,2839 ****
--- 2834,2934 ----
  -------
  (0 rows)
  
+ -- check some degenerate cases
+ create temp table t1 (f1 int, f2 int8);
+ insert into t1 values (1,1),(1,2),(2,2);
+ select f1, sum(f1) over (partition by f1
+                          range between 1 preceding and 1 following)
+ from t1 where f1 = f2;  -- error, must have order by
+ ERROR:  RANGE with offset PRECEDING/FOLLOWING requires exactly one ORDER BY column
+ LINE 1: select f1, sum(f1) over (partition by f1
+                                 ^
+ explain (costs off)
+ select f1, sum(f1) over (partition by f1 order by f2
+                          range between 1 preceding and 1 following)
+ from t1 where f1 = f2;
+            QUERY PLAN            
+ ---------------------------------
+  WindowAgg
+    ->  Sort
+          Sort Key: f1
+          ->  Seq Scan on t1
+                Filter: (f1 = f2)
+ (5 rows)
+ 
+ select f1, sum(f1) over (partition by f1 order by f2
+                          range between 1 preceding and 1 following)
+ from t1 where f1 = f2;
+  f1 | sum 
+ ----+-----
+   1 |   1
+   2 |   2
+ (2 rows)
+ 
+ select f1, sum(f1) over (partition by f1, f1 order by f2
+                          range between 2 preceding and 1 preceding)
+ from t1 where f1 = f2;
+  f1 | sum 
+ ----+-----
+   1 |    
+   2 |    
+ (2 rows)
+ 
+ select f1, sum(f1) over (partition by f1, f2 order by f2
+                          range between 1 following and 2 following)
+ from t1 where f1 = f2;
+  f1 | sum 
+ ----+-----
+   1 |    
+   2 |    
+ (2 rows)
+ 
+ select f1, sum(f1) over (partition by f1
+                          groups between 1 preceding and 1 following)
+ from t1 where f1 = f2;  -- error, must have order by
+ ERROR:  GROUPS mode requires an ORDER BY clause
+ LINE 1: select f1, sum(f1) over (partition by f1
+                                 ^
+ explain (costs off)
+ select f1, sum(f1) over (partition by f1 order by f2
+                          groups between 1 preceding and 1 following)
+ from t1 where f1 = f2;
+            QUERY PLAN            
+ ---------------------------------
+  WindowAgg
+    ->  Sort
+          Sort Key: f1
+          ->  Seq Scan on t1
+                Filter: (f1 = f2)
+ (5 rows)
+ 
+ select f1, sum(f1) over (partition by f1 order by f2
+                          groups between 1 preceding and 1 following)
+ from t1 where f1 = f2;
+  f1 | sum 
+ ----+-----
+   1 |   1
+   2 |   2
+ (2 rows)
+ 
+ select f1, sum(f1) over (partition by f1, f1 order by f2
+                          groups between 2 preceding and 1 preceding)
+ from t1 where f1 = f2;
+  f1 | sum 
+ ----+-----
+   1 |    
+   2 |    
+ (2 rows)
+ 
+ select f1, sum(f1) over (partition by f1, f2 order by f2
+                          groups between 1 following and 2 following)
+ from t1 where f1 = f2;
+  f1 | sum 
+ ----+-----
+   1 |    
+   2 |    
+ (2 rows)
+ 
  -- ordering by a non-integer constant is allowed
  SELECT rank() OVER (ORDER BY length('abc'));
   rank 
diff --git a/src/test/regress/sql/window.sql b/src/test/regress/sql/window.sql
index 051b50b..e2943a3 100644
*** a/src/test/regress/sql/window.sql
--- b/src/test/regress/sql/window.sql
*************** WINDOW w AS (ORDER BY x groups between 1
*** 795,800 ****
--- 795,838 ----
  -- with UNION
  SELECT count(*) OVER (PARTITION BY four) FROM (SELECT * FROM tenk1 UNION ALL SELECT * FROM tenk2)s LIMIT 0;
  
+ -- check some degenerate cases
+ create temp table t1 (f1 int, f2 int8);
+ insert into t1 values (1,1),(1,2),(2,2);
+ 
+ select f1, sum(f1) over (partition by f1
+                          range between 1 preceding and 1 following)
+ from t1 where f1 = f2;  -- error, must have order by
+ explain (costs off)
+ select f1, sum(f1) over (partition by f1 order by f2
+                          range between 1 preceding and 1 following)
+ from t1 where f1 = f2;
+ select f1, sum(f1) over (partition by f1 order by f2
+                          range between 1 preceding and 1 following)
+ from t1 where f1 = f2;
+ select f1, sum(f1) over (partition by f1, f1 order by f2
+                          range between 2 preceding and 1 preceding)
+ from t1 where f1 = f2;
+ select f1, sum(f1) over (partition by f1, f2 order by f2
+                          range between 1 following and 2 following)
+ from t1 where f1 = f2;
+ 
+ select f1, sum(f1) over (partition by f1
+                          groups between 1 preceding and 1 following)
+ from t1 where f1 = f2;  -- error, must have order by
+ explain (costs off)
+ select f1, sum(f1) over (partition by f1 order by f2
+                          groups between 1 preceding and 1 following)
+ from t1 where f1 = f2;
+ select f1, sum(f1) over (partition by f1 order by f2
+                          groups between 1 preceding and 1 following)
+ from t1 where f1 = f2;
+ select f1, sum(f1) over (partition by f1, f1 order by f2
+                          groups between 2 preceding and 1 preceding)
+ from t1 where f1 = f2;
+ select f1, sum(f1) over (partition by f1, f2 order by f2
+                          groups between 1 following and 2 following)
+ from t1 where f1 = f2;
+ 
  -- ordering by a non-integer constant is allowed
  SELECT rank() OVER (ORDER BY length('abc'));