I wrote:
> The only alternative I can see is to make a back-patch that just teaches
> get_eclass_for_sort_expr() to compute valid nullable_relids for the sort
> expression.  That's necessary code in any case, and it would fix the
> immediately complained-of bug.  The thing that scares me is that it is
> now clear that equivclass.c is capable of considering two expressions to
> be equivalent when they should not be; that is, I'm afraid there are
> variants of the problem that would not be cured by such a limited
> back-patch.  But maybe I should try to create such an example before
> proposing the more invasive approach.

In thinking about how to compute valid nullable_relids in
get_eclass_for_sort_expr, I realized that it is impractical to do it
in the current code structure, because computing nullable_relids
requires that the joininfo list has been built by deconstruct_jointree,
which hasn't been run yet at the time grouping_planner computes the
pathkeys for the ORDER BY and related clauses.  So I concluded that
we need to postpone computation of those pathkeys lists until after
deconstruct_jointree.

Working through this, I realized that we could, in fact, simply delay
computation of all five of group_pathkeys, window_pathkeys,
distinct_pathkeys, sort_pathkeys, and query_pathkeys until the point
where we currently do canonicalize_all_pathkeys.  There isn't anything
that looks at those lists in-between.  The only reason they're set up by
grouping_planner is separation of concerns --- query_planner doesn't
really have any business dealing with group_pathkeys for instance.
Furthermore, if we delay computation of those lists, then we can get rid
of the notion of non-canonical pathkeys altogether: there is noplace
that needs to generate a PathKey at all before this point.  So that
results in a nice conceptual simplification as well as saving a few
cycles.

Once I'd finished moving the pathkeys calculations, I was a tad
surprised to discover that bug 8049 was gone!  The reason for this is
that now, if an ORDER BY item matches some expression that has another
reason to be in an EquivalenceClass, the ORDER BY item will get matched
to a pre-existing EC item and so there will be no opportunity for
get_eclass_for_sort_expr() to do the wrong thing.  If an ORDER BY item
doesn't match anything, then it will be created as a new single-item
EquivalenceClass, containing arguably-incorrect NULL em_nullable_relids,
but there is nothing that will notice that.

I remain suspicious that there are or will someday be cases where we
actually need to generate valid em_nullable_relids for an ORDER BY item,
but in the absence of a demonstrated bug it doesn't seem real prudent to
add a bunch of new code here, either in 9.2 or 9.3.  Therefore, I think
what we ought to do is apply something like the attached patch to 9.2
and 9.3 and call it a day for now.

The difficulty with back-patching this patch is that it involves API
changes for query_planner() and make_pathkeys_for_sortclauses(), as well
as outright removal of canonicalize_pathkeys().  So there's a risk of
breaking third-party code that calls any of those functions.  I don't
have the slightest hesitation about changing these APIs in 9.3, but
back-patching them into 9.2 is a bit more nervous-making.

It would be trivial to make make_pathkeys_for_sortclauses() ignore
its "canonicalize" parameter in 9.2, or maybe better add an Assert
(not a runtime test) that the parameter is "true".  And we could
turn canonicalize_pathkeys() into a no-op function in 9.2.  However,
I'm not seeing a better alternative to adding the callback parameter
to query_planner().  The difficulty is that in order to compute those
pathkey lists, we need access to the "tlist" and "activeWindows" values,
which heretofore have just been local in grouping_planner().  We could
add fields to PlannerInfo to carry them, and then grouping_planner()
could just call some new function in planner.c rather than needing an
explicit callback argument.  But new fields in PlannerInfo are an ABI
break that in my judgment would be more risky in 9.2 than changing
query_planner()'s signature --- I think it's somewhat unlikely that
any plug-ins are calling query_planner() directly.

Thoughts?  Anybody know of a counterexample to the idea that no plug-ins
call query_planner()?

                        regards, tom lane

diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 7245fa32a0f0d7d7bb924eb8461d68a2a0d702ed..3f96595e8eb041bcebe38639e075db7eb8a77fd2 100644
*** a/src/backend/nodes/equalfuncs.c
--- b/src/backend/nodes/equalfuncs.c
*************** _equalFromExpr(const FromExpr *a, const 
*** 728,749 ****
  static bool
  _equalPathKey(const PathKey *a, const PathKey *b)
  {
! 	/*
! 	 * This is normally used on non-canonicalized PathKeys, so must chase up
! 	 * to the topmost merged EquivalenceClass and see if those are the same
! 	 * (by pointer equality).
! 	 */
! 	EquivalenceClass *a_eclass;
! 	EquivalenceClass *b_eclass;
! 
! 	a_eclass = a->pk_eclass;
! 	while (a_eclass->ec_merged)
! 		a_eclass = a_eclass->ec_merged;
! 	b_eclass = b->pk_eclass;
! 	while (b_eclass->ec_merged)
! 		b_eclass = b_eclass->ec_merged;
! 	if (a_eclass != b_eclass)
! 		return false;
  	COMPARE_SCALAR_FIELD(pk_opfamily);
  	COMPARE_SCALAR_FIELD(pk_strategy);
  	COMPARE_SCALAR_FIELD(pk_nulls_first);
--- 728,735 ----
  static bool
  _equalPathKey(const PathKey *a, const PathKey *b)
  {
! 	/* We assume pointer equality is sufficient to compare the eclasses */
! 	COMPARE_SCALAR_FIELD(pk_eclass);
  	COMPARE_SCALAR_FIELD(pk_opfamily);
  	COMPARE_SCALAR_FIELD(pk_strategy);
  	COMPARE_SCALAR_FIELD(pk_nulls_first);
diff --git a/src/backend/optimizer/README b/src/backend/optimizer/README
index 438c2ca1826da788d4b40ecaa9d51006efb6c985..751766fb9dd604e40700e76ef5b8ae3b6a6dd469 100644
*** a/src/backend/optimizer/README
--- b/src/backend/optimizer/README
*************** since they are easily compared to the pa
*** 589,604 ****
  path.  So, SortGroupClause lists are turned into pathkeys lists for use
  inside the optimizer.
  
- Because we have to generate pathkeys lists from the sort clauses before
- we've finished EquivalenceClass merging, we cannot use the pointer-equality
- method of comparing PathKeys in the earliest stages of the planning
- process.  Instead, we generate "non canonical" PathKeys that reference
- single-element EquivalenceClasses that might get merged later.  After we
- complete EquivalenceClass merging, we replace these with "canonical"
- PathKeys that reference only fully-merged classes, and after that we make
- sure we don't generate more than one copy of each "canonical" PathKey.
- Then it is safe to use pointer comparison on canonical PathKeys.
- 
  An additional refinement we can make is to insist that canonical pathkey
  lists (sort orderings) do not mention the same EquivalenceClass more than
  once.  For example, in all these cases the second sort column is redundant,
--- 589,594 ----
*************** mergejoinable clauses found in the quals
*** 651,662 ****
  we know all we can know about equivalence of different variables, so
  subsequently there will be no further merging of EquivalenceClasses.
  At that point it is possible to consider the EquivalenceClasses as
! "canonical" and build canonical PathKeys that reference them.  Before
! we reach that point (actually, before entering query_planner at all)
! we also ensure that we have constructed EquivalenceClasses for all the
! expressions used in the query's ORDER BY and related clauses.  These
! classes might or might not get merged together, depending on what we
! find in the quals.
  
  Because all the EquivalenceClasses are known before we begin path
  generation, we can use them as a guide to which indexes are of interest:
--- 641,651 ----
  we know all we can know about equivalence of different variables, so
  subsequently there will be no further merging of EquivalenceClasses.
  At that point it is possible to consider the EquivalenceClasses as
! "canonical" and build canonical PathKeys that reference them.  At this
! time we construct PathKeys for the query's ORDER BY and related clauses.
! (Any ordering expressions that do not appear elsewhere will result in
! the creation of new EquivalenceClasses, but this cannot result in merging
! existing classes, so canonical-ness is not lost.)
  
  Because all the EquivalenceClasses are known before we begin path
  generation, we can use them as a guide to which indexes are of interest:
diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c
index 5c4ac066a52838488d88583907ecfe7cef473acb..cbb4f5cd95604e1cffa52b6563c089f10e868511 100644
*** a/src/backend/optimizer/path/equivclass.c
--- b/src/backend/optimizer/path/equivclass.c
*************** process_equivalence(PlannerInfo *root, R
*** 285,295 ****
  		}
  
  		/*
! 		 * Case 2: need to merge ec1 and ec2.  We add ec2's items to ec1, then
! 		 * set ec2's ec_merged link to point to ec1 and remove ec2 from the
! 		 * eq_classes list.  We cannot simply delete ec2 because that could
! 		 * leave dangling pointers in existing PathKeys.  We leave it behind
! 		 * with a link so that the merged EC can be found.
  		 */
  		ec1->ec_members = list_concat(ec1->ec_members, ec2->ec_members);
  		ec1->ec_sources = list_concat(ec1->ec_sources, ec2->ec_sources);
--- 285,303 ----
  		}
  
  		/*
! 		 * Case 2: need to merge ec1 and ec2.  This should never happen after
! 		 * we've built any canonical pathkeys; if it did, those pathkeys might
! 		 * be rendered non-canonical by the merge.
! 		 */
! 		if (root->canon_pathkeys != NIL)
! 			elog(ERROR, "too late to merge equivalence classes");
! 
! 		/*
! 		 * We add ec2's items to ec1, then set ec2's ec_merged link to point
! 		 * to ec1 and remove ec2 from the eq_classes list.  We cannot simply
! 		 * delete ec2 because that could leave dangling pointers in existing
! 		 * PathKeys.  We leave it behind with a link so that the merged EC can
! 		 * be found.
  		 */
  		ec1->ec_members = list_concat(ec1->ec_members, ec2->ec_members);
  		ec1->ec_sources = list_concat(ec1->ec_sources, ec2->ec_sources);
diff --git a/src/backend/optimizer/path/pathkeys.c b/src/backend/optimizer/path/pathkeys.c
index c52fd1e7af8451f8c79bbdbc87ca674290d85d72..672499691951f36c9a215ea0c5e0f7ecd039db97 100644
*** a/src/backend/optimizer/path/pathkeys.c
--- b/src/backend/optimizer/path/pathkeys.c
***************
*** 28,35 ****
  #include "utils/lsyscache.h"
  
  
- static PathKey *makePathKey(EquivalenceClass *eclass, Oid opfamily,
- 			int strategy, bool nulls_first);
  static PathKey *make_canonical_pathkey(PlannerInfo *root,
  					   EquivalenceClass *eclass, Oid opfamily,
  					   int strategy, bool nulls_first);
--- 28,33 ----
*************** static bool right_merge_direction(Planne
*** 42,75 ****
   ****************************************************************************/
  
  /*
-  * makePathKey
-  *		create a PathKey node
-  *
-  * This does not promise to create a canonical PathKey, it's merely a
-  * convenience routine to build the specified node.
-  */
- static PathKey *
- makePathKey(EquivalenceClass *eclass, Oid opfamily,
- 			int strategy, bool nulls_first)
- {
- 	PathKey    *pk = makeNode(PathKey);
- 
- 	pk->pk_eclass = eclass;
- 	pk->pk_opfamily = opfamily;
- 	pk->pk_strategy = strategy;
- 	pk->pk_nulls_first = nulls_first;
- 
- 	return pk;
- }
- 
- /*
   * make_canonical_pathkey
   *	  Given the parameters for a PathKey, find any pre-existing matching
   *	  pathkey in the query's list of "canonical" pathkeys.  Make a new
   *	  entry if there's not one already.
   *
   * Note that this function must not be used until after we have completed
!  * merging EquivalenceClasses.
   */
  static PathKey *
  make_canonical_pathkey(PlannerInfo *root,
--- 40,54 ----
   ****************************************************************************/
  
  /*
   * make_canonical_pathkey
   *	  Given the parameters for a PathKey, find any pre-existing matching
   *	  pathkey in the query's list of "canonical" pathkeys.  Make a new
   *	  entry if there's not one already.
   *
   * Note that this function must not be used until after we have completed
!  * merging EquivalenceClasses.	(We don't try to enforce that here; instead,
!  * equivclass.c will complain if a merge occurs after root->canon_pathkeys
!  * has become nonempty.)
   */
  static PathKey *
  make_canonical_pathkey(PlannerInfo *root,
*************** make_canonical_pathkey(PlannerInfo *root
*** 100,106 ****
  	 */
  	oldcontext = MemoryContextSwitchTo(root->planner_cxt);
  
! 	pk = makePathKey(eclass, opfamily, strategy, nulls_first);
  	root->canon_pathkeys = lappend(root->canon_pathkeys, pk);
  
  	MemoryContextSwitchTo(oldcontext);
--- 79,90 ----
  	 */
  	oldcontext = MemoryContextSwitchTo(root->planner_cxt);
  
! 	pk = makeNode(PathKey);
! 	pk->pk_eclass = eclass;
! 	pk->pk_opfamily = opfamily;
! 	pk->pk_strategy = strategy;
! 	pk->pk_nulls_first = nulls_first;
! 
  	root->canon_pathkeys = lappend(root->canon_pathkeys, pk);
  
  	MemoryContextSwitchTo(oldcontext);
*************** make_canonical_pathkey(PlannerInfo *root
*** 112,119 ****
   * pathkey_is_redundant
   *	   Is a pathkey redundant with one already in the given list?
   *
!  * Both the given pathkey and the list members must be canonical for this
!  * to work properly.  We detect two cases:
   *
   * 1. If the new pathkey's equivalence class contains a constant, and isn't
   * below an outer join, then we can disregard it as a sort key.  An example:
--- 96,102 ----
   * pathkey_is_redundant
   *	   Is a pathkey redundant with one already in the given list?
   *
!  * We detect two cases:
   *
   * 1. If the new pathkey's equivalence class contains a constant, and isn't
   * below an outer join, then we can disregard it as a sort key.  An example:
*************** make_canonical_pathkey(PlannerInfo *root
*** 135,140 ****
--- 118,129 ----
   * Note in particular that we need not compare opfamily (all the opfamilies
   * of the EC have the same notion of equality) nor sort direction.
   *
+  * Both the given pathkey and the list members must be canonical for this
+  * to work properly, but that's okay since we no longer ever construct any
+  * non-canonical pathkeys.	(Note: the notion of a pathkey *list* being
+  * canonical includes the additional requirement of no redundant entries,
+  * which is exactly what we are checking for here.)
+  *
   * Because the equivclass.c machinery forms only one copy of any EC per query,
   * pointer comparison is enough to decide whether canonical ECs are the same.
   */
*************** pathkey_is_redundant(PathKey *new_pathke
*** 144,152 ****
  	EquivalenceClass *new_ec = new_pathkey->pk_eclass;
  	ListCell   *lc;
  
- 	/* Assert we've been given canonical pathkeys */
- 	Assert(!new_ec->ec_merged);
- 
  	/* Check for EC containing a constant --- unconditionally redundant */
  	if (EC_MUST_BE_REDUNDANT(new_ec))
  		return true;
--- 133,138 ----
*************** pathkey_is_redundant(PathKey *new_pathke
*** 156,164 ****
  	{
  		PathKey    *old_pathkey = (PathKey *) lfirst(lc);
  
- 		/* Assert we've been given canonical pathkeys */
- 		Assert(!old_pathkey->pk_eclass->ec_merged);
- 
  		if (new_ec == old_pathkey->pk_eclass)
  			return true;
  	}
--- 142,147 ----
*************** pathkey_is_redundant(PathKey *new_pathke
*** 167,222 ****
  }
  
  /*
-  * canonicalize_pathkeys
-  *	   Convert a not-necessarily-canonical pathkeys list to canonical form.
-  *
-  * Note that this function must not be used until after we have completed
-  * merging EquivalenceClasses.
-  */
- List *
- canonicalize_pathkeys(PlannerInfo *root, List *pathkeys)
- {
- 	List	   *new_pathkeys = NIL;
- 	ListCell   *l;
- 
- 	foreach(l, pathkeys)
- 	{
- 		PathKey    *pathkey = (PathKey *) lfirst(l);
- 		EquivalenceClass *eclass;
- 		PathKey    *cpathkey;
- 
- 		/* Find the canonical (merged) EquivalenceClass */
- 		eclass = pathkey->pk_eclass;
- 		while (eclass->ec_merged)
- 			eclass = eclass->ec_merged;
- 
- 		/*
- 		 * If we can tell it's redundant just from the EC, skip.
- 		 * pathkey_is_redundant would notice that, but we needn't even bother
- 		 * constructing the node...
- 		 */
- 		if (EC_MUST_BE_REDUNDANT(eclass))
- 			continue;
- 
- 		/* OK, build a canonicalized PathKey struct */
- 		cpathkey = make_canonical_pathkey(root,
- 										  eclass,
- 										  pathkey->pk_opfamily,
- 										  pathkey->pk_strategy,
- 										  pathkey->pk_nulls_first);
- 
- 		/* Add to list unless redundant */
- 		if (!pathkey_is_redundant(cpathkey, new_pathkeys))
- 			new_pathkeys = lappend(new_pathkeys, cpathkey);
- 	}
- 	return new_pathkeys;
- }
- 
- /*
   * make_pathkey_from_sortinfo
   *	  Given an expression and sort-order information, create a PathKey.
!  *	  If canonicalize = true, the result is a "canonical" PathKey,
!  *	  otherwise not.  (But note it might be redundant anyway.)
   *
   * If the PathKey is being generated from a SortGroupClause, sortref should be
   * the SortGroupClause's SortGroupRef; otherwise zero.
--- 150,158 ----
  }
  
  /*
   * make_pathkey_from_sortinfo
   *	  Given an expression and sort-order information, create a PathKey.
!  *	  The result is always a "canonical" PathKey, but it might be redundant.
   *
   * If the PathKey is being generated from a SortGroupClause, sortref should be
   * the SortGroupClause's SortGroupRef; otherwise zero.
*************** canonicalize_pathkeys(PlannerInfo *root,
*** 229,237 ****
   * create_it is TRUE if we should create any missing EquivalenceClass
   * needed to represent the sort key.  If it's FALSE, we return NULL if the
   * sort key isn't already present in any EquivalenceClass.
-  *
-  * canonicalize should always be TRUE after EquivalenceClass merging has
-  * been performed, but FALSE if we haven't done EquivalenceClass merging yet.
   */
  static PathKey *
  make_pathkey_from_sortinfo(PlannerInfo *root,
--- 165,170 ----
*************** make_pathkey_from_sortinfo(PlannerInfo *
*** 243,250 ****
  						   bool nulls_first,
  						   Index sortref,
  						   Relids rel,
! 						   bool create_it,
! 						   bool canonicalize)
  {
  	int16		strategy;
  	Oid			equality_op;
--- 176,182 ----
  						   bool nulls_first,
  						   Index sortref,
  						   Relids rel,
! 						   bool create_it)
  {
  	int16		strategy;
  	Oid			equality_op;
*************** make_pathkey_from_sortinfo(PlannerInfo *
*** 281,291 ****
  		return NULL;
  
  	/* And finally we can find or create a PathKey node */
! 	if (canonicalize)
! 		return make_canonical_pathkey(root, eclass, opfamily,
! 									  strategy, nulls_first);
! 	else
! 		return makePathKey(eclass, opfamily, strategy, nulls_first);
  }
  
  /*
--- 213,220 ----
  		return NULL;
  
  	/* And finally we can find or create a PathKey node */
! 	return make_canonical_pathkey(root, eclass, opfamily,
! 								  strategy, nulls_first);
  }
  
  /*
*************** make_pathkey_from_sortop(PlannerInfo *ro
*** 301,308 ****
  						 Oid ordering_op,
  						 bool nulls_first,
  						 Index sortref,
! 						 bool create_it,
! 						 bool canonicalize)
  {
  	Oid			opfamily,
  				opcintype,
--- 230,236 ----
  						 Oid ordering_op,
  						 bool nulls_first,
  						 Index sortref,
! 						 bool create_it)
  {
  	Oid			opfamily,
  				opcintype,
*************** make_pathkey_from_sortop(PlannerInfo *ro
*** 327,334 ****
  									  nulls_first,
  									  sortref,
  									  NULL,
! 									  create_it,
! 									  canonicalize);
  }
  
  
--- 255,261 ----
  									  nulls_first,
  									  sortref,
  									  NULL,
! 									  create_it);
  }
  
  
*************** make_pathkey_from_sortop(PlannerInfo *ro
*** 341,349 ****
   *	  Compare two pathkeys to see if they are equivalent, and if not whether
   *	  one is "better" than the other.
   *
!  *	  This function may only be applied to canonicalized pathkey lists.
!  *	  In the canonical representation, pathkeys can be checked for equality
!  *	  by simple pointer comparison.
   */
  PathKeysComparison
  compare_pathkeys(List *keys1, List *keys2)
--- 268,275 ----
   *	  Compare two pathkeys to see if they are equivalent, and if not whether
   *	  one is "better" than the other.
   *
!  *	  We assume the pathkeys are canonical, and so they can be checked for
!  *	  equality by simple pointer comparison.
   */
  PathKeysComparison
  compare_pathkeys(List *keys1, List *keys2)
*************** compare_pathkeys(List *keys1, List *keys
*** 364,378 ****
  		PathKey    *pathkey1 = (PathKey *) lfirst(key1);
  		PathKey    *pathkey2 = (PathKey *) lfirst(key2);
  
- 		/*
- 		 * XXX would like to check that we've been given canonicalized input,
- 		 * but PlannerInfo not accessible here...
- 		 */
- #ifdef NOT_USED
- 		Assert(list_member_ptr(root->canon_pathkeys, pathkey1));
- 		Assert(list_member_ptr(root->canon_pathkeys, pathkey2));
- #endif
- 
  		if (pathkey1 != pathkey2)
  			return PATHKEYS_DIFFERENT;	/* no need to keep looking */
  	}
--- 290,295 ----
*************** pathkeys_contained_in(List *keys1, List 
*** 414,420 ****
   *	  Return NULL if no such path.
   *
   * 'paths' is a list of possible paths that all generate the same relation
!  * 'pathkeys' represents a required ordering (already canonicalized!)
   * 'required_outer' denotes allowable outer relations for parameterized paths
   * 'cost_criterion' is STARTUP_COST or TOTAL_COST
   */
--- 331,337 ----
   *	  Return NULL if no such path.
   *
   * 'paths' is a list of possible paths that all generate the same relation
!  * 'pathkeys' represents a required ordering (in canonical form!)
   * 'required_outer' denotes allowable outer relations for parameterized paths
   * 'cost_criterion' is STARTUP_COST or TOTAL_COST
   */
*************** get_cheapest_path_for_pathkeys(List *pat
*** 455,461 ****
   * parameter.
   *
   * 'paths' is a list of possible paths that all generate the same relation
!  * 'pathkeys' represents a required ordering (already canonicalized!)
   * 'required_outer' denotes allowable outer relations for parameterized paths
   * 'fraction' is the fraction of the total tuples expected to be retrieved
   */
--- 372,378 ----
   * parameter.
   *
   * 'paths' is a list of possible paths that all generate the same relation
!  * 'pathkeys' represents a required ordering (in canonical form!)
   * 'required_outer' denotes allowable outer relations for parameterized paths
   * 'fraction' is the fraction of the total tuples expected to be retrieved
   */
*************** build_index_pathkeys(PlannerInfo *root,
*** 554,561 ****
  											  nulls_first,
  											  0,
  											  index->rel->relids,
! 											  false,
! 											  true);
  
  		/*
  		 * If the sort key isn't already present in any EquivalenceClass, then
--- 471,477 ----
  											  nulls_first,
  											  0,
  											  index->rel->relids,
! 											  false);
  
  		/*
  		 * If the sort key isn't already present in any EquivalenceClass, then
*************** build_join_pathkeys(PlannerInfo *root,
*** 829,840 ****
   *		Generate a pathkeys list that represents the sort order specified
   *		by a list of SortGroupClauses
   *
!  * If canonicalize is TRUE, the resulting PathKeys are all in canonical form;
!  * otherwise not.  canonicalize should always be TRUE after EquivalenceClass
!  * merging has been performed, but FALSE if we haven't done EquivalenceClass
!  * merging yet.  (We provide this option because grouping_planner() needs to
!  * be able to represent requested pathkeys before the equivalence classes have
!  * been created for the query.)
   *
   * 'sortclauses' is a list of SortGroupClause nodes
   * 'tlist' is the targetlist to find the referenced tlist entries in
--- 745,752 ----
   *		Generate a pathkeys list that represents the sort order specified
   *		by a list of SortGroupClauses
   *
!  * The resulting PathKeys are always in canonical form.  (Actually, there
!  * is no longer any code anywhere that creates non-canonical PathKeys.)
   *
   * 'sortclauses' is a list of SortGroupClause nodes
   * 'tlist' is the targetlist to find the referenced tlist entries in
*************** build_join_pathkeys(PlannerInfo *root,
*** 842,849 ****
  List *
  make_pathkeys_for_sortclauses(PlannerInfo *root,
  							  List *sortclauses,
! 							  List *tlist,
! 							  bool canonicalize)
  {
  	List	   *pathkeys = NIL;
  	ListCell   *l;
--- 754,760 ----
  List *
  make_pathkeys_for_sortclauses(PlannerInfo *root,
  							  List *sortclauses,
! 							  List *tlist)
  {
  	List	   *pathkeys = NIL;
  	ListCell   *l;
*************** make_pathkeys_for_sortclauses(PlannerInf
*** 861,876 ****
  										   sortcl->sortop,
  										   sortcl->nulls_first,
  										   sortcl->tleSortGroupRef,
! 										   true,
! 										   canonicalize);
  
  		/* Canonical form eliminates redundant ordering keys */
! 		if (canonicalize)
! 		{
! 			if (!pathkey_is_redundant(pathkey, pathkeys))
! 				pathkeys = lappend(pathkeys, pathkey);
! 		}
! 		else
  			pathkeys = lappend(pathkeys, pathkey);
  	}
  	return pathkeys;
--- 772,781 ----
  										   sortcl->sortop,
  										   sortcl->nulls_first,
  										   sortcl->tleSortGroupRef,
! 										   true);
  
  		/* Canonical form eliminates redundant ordering keys */
! 		if (!pathkey_is_redundant(pathkey, pathkeys))
  			pathkeys = lappend(pathkeys, pathkey);
  	}
  	return pathkeys;
diff --git a/src/backend/optimizer/plan/planagg.c b/src/backend/optimizer/plan/planagg.c
index 4007ac455880d110c283888655be28c84bf84691..5bbfd2377eb7ce87aefef1c30d8f9554c9852cca 100644
*** a/src/backend/optimizer/plan/planagg.c
--- b/src/backend/optimizer/plan/planagg.c
***************
*** 48,53 ****
--- 48,54 ----
  static bool find_minmax_aggs_walker(Node *node, List **context);
  static bool build_minmax_path(PlannerInfo *root, MinMaxAggInfo *mminfo,
  				  Oid eqop, Oid sortop, bool nulls_first);
+ static void minmax_qp_callback(PlannerInfo *root, void *extra);
  static void make_agg_subplan(PlannerInfo *root, MinMaxAggInfo *mminfo);
  static Node *replace_aggs_with_params_mutator(Node *node, PlannerInfo *root);
  static Oid	fetch_agg_sort_op(Oid aggfnoid);
*************** build_minmax_path(PlannerInfo *root, Min
*** 446,470 ****
  										   FLOAT8PASSBYVAL);
  
  	/*
- 	 * Set up requested pathkeys.
- 	 */
- 	subroot->group_pathkeys = NIL;
- 	subroot->window_pathkeys = NIL;
- 	subroot->distinct_pathkeys = NIL;
- 
- 	subroot->sort_pathkeys =
- 		make_pathkeys_for_sortclauses(subroot,
- 									  parse->sortClause,
- 									  parse->targetList,
- 									  false);
- 
- 	subroot->query_pathkeys = subroot->sort_pathkeys;
- 
- 	/*
  	 * Generate the best paths for this query, telling query_planner that we
  	 * have LIMIT 1.
  	 */
  	query_planner(subroot, parse->targetList, 1.0, 1.0,
  				  &cheapest_path, &sorted_path, &dNumGroups);
  
  	/*
--- 447,457 ----
  										   FLOAT8PASSBYVAL);
  
  	/*
  	 * Generate the best paths for this query, telling query_planner that we
  	 * have LIMIT 1.
  	 */
  	query_planner(subroot, parse->targetList, 1.0, 1.0,
+ 				  minmax_qp_callback, NULL,
  				  &cheapest_path, &sorted_path, &dNumGroups);
  
  	/*
*************** build_minmax_path(PlannerInfo *root, Min
*** 506,511 ****
--- 493,516 ----
  }
  
  /*
+  * Compute query_pathkeys and other pathkeys during plan generation
+  */
+ static void
+ minmax_qp_callback(PlannerInfo *root, void *extra)
+ {
+ 	root->group_pathkeys = NIL;
+ 	root->window_pathkeys = NIL;
+ 	root->distinct_pathkeys = NIL;
+ 
+ 	root->sort_pathkeys =
+ 		make_pathkeys_for_sortclauses(root,
+ 									  root->parse->sortClause,
+ 									  root->parse->targetList);
+ 
+ 	root->query_pathkeys = root->sort_pathkeys;
+ }
+ 
+ /*
   * Construct a suitable plan for a converted aggregate query
   */
  static void
diff --git a/src/backend/optimizer/plan/planmain.c b/src/backend/optimizer/plan/planmain.c
index a919914f944b3e89f1127d9da94f4bdf0b7451ee..42a98945a38d07972ee2e8b5de16b3639196eebd 100644
*** a/src/backend/optimizer/plan/planmain.c
--- b/src/backend/optimizer/plan/planmain.c
***************
*** 30,39 ****
  #include "utils/selfuncs.h"
  
  
- /* Local functions */
- static void canonicalize_all_pathkeys(PlannerInfo *root);
- 
- 
  /*
   * query_planner
   *	  Generate a path (that is, a simplified plan) for a basic query,
--- 30,35 ----
*************** static void canonicalize_all_pathkeys(Pl
*** 55,60 ****
--- 51,58 ----
   * tuple_fraction is the fraction of tuples we expect will be retrieved
   * limit_tuples is a hard limit on number of tuples to retrieve,
   *		or -1 if no limit
+  * qp_callback is a function to compute query_pathkeys once it's safe to do so
+  * qp_extra is optional extra data to pass to qp_callback
   *
   * Output parameters:
   * *cheapest_path receives the overall-cheapest path for the query
*************** static void canonicalize_all_pathkeys(Pl
*** 63,80 ****
   * *num_groups receives the estimated number of groups, or 1 if query
   *				does not use grouping
   *
!  * Note: the PlannerInfo node also includes a query_pathkeys field, which is
!  * both an input and an output of query_planner().	The input value signals
!  * query_planner that the indicated sort order is wanted in the final output
!  * plan.  But this value has not yet been "canonicalized", since the needed
!  * info does not get computed until we scan the qual clauses.  We canonicalize
!  * it as soon as that task is done.  (The main reason query_pathkeys is a
!  * PlannerInfo field and not a passed parameter is that the low-level routines
!  * in indxpath.c need to see it.)
!  *
!  * Note: the PlannerInfo node includes other pathkeys fields besides
!  * query_pathkeys, all of which need to be canonicalized once the info is
!  * available.  See canonicalize_all_pathkeys.
   *
   * tuple_fraction is interpreted as follows:
   *	  0: expect all tuples to be retrieved (normal case)
--- 61,71 ----
   * *num_groups receives the estimated number of groups, or 1 if query
   *				does not use grouping
   *
!  * Note: the PlannerInfo node also includes a query_pathkeys field, which
!  * tells query_planner the sort order that is desired in the final output
!  * plan.  This value is *not* available at call time, but is computed by
!  * qp_callback once we have completed merging the query's equivalence classes.
!  * (We cannot construct canonical pathkeys until that's done.)
   *
   * tuple_fraction is interpreted as follows:
   *	  0: expect all tuples to be retrieved (normal case)
*************** static void canonicalize_all_pathkeys(Pl
*** 89,94 ****
--- 80,86 ----
  void
  query_planner(PlannerInfo *root, List *tlist,
  			  double tuple_fraction, double limit_tuples,
+ 			  query_pathkeys_callback qp_callback, void *qp_extra,
  			  Path **cheapest_path, Path **sorted_path,
  			  double *num_groups)
  {
*************** query_planner(PlannerInfo *root, List *t
*** 118,128 ****
  		*sorted_path = NULL;
  
  		/*
! 		 * We still are required to canonicalize any pathkeys, in case it's
! 		 * something like "SELECT 2+2 ORDER BY 1".
  		 */
  		root->canon_pathkeys = NIL;
! 		canonicalize_all_pathkeys(root);
  		return;
  	}
  
--- 110,120 ----
  		*sorted_path = NULL;
  
  		/*
! 		 * We still are required to call qp_callback, in case it's something
! 		 * like "SELECT 2+2 ORDER BY 1".
  		 */
  		root->canon_pathkeys = NIL;
! 		(*qp_callback) (root, qp_extra);
  		return;
  	}
  
*************** query_planner(PlannerInfo *root, List *t
*** 205,214 ****
  
  	/*
  	 * We have completed merging equivalence sets, so it's now possible to
! 	 * convert previously generated pathkeys (in particular, the requested
! 	 * query_pathkeys) to canonical form.
  	 */
! 	canonicalize_all_pathkeys(root);
  
  	/*
  	 * Examine any "placeholder" expressions generated during subquery pullup.
--- 197,206 ----
  
  	/*
  	 * We have completed merging equivalence sets, so it's now possible to
! 	 * generate pathkeys in canonical form; so compute query_pathkeys and
! 	 * other pathkeys fields in PlannerInfo.
  	 */
! 	(*qp_callback) (root, qp_extra);
  
  	/*
  	 * Examine any "placeholder" expressions generated during subquery pullup.
*************** query_planner(PlannerInfo *root, List *t
*** 429,447 ****
  	*cheapest_path = cheapestpath;
  	*sorted_path = sortedpath;
  }
- 
- 
- /*
-  * canonicalize_all_pathkeys
-  *		Canonicalize all pathkeys that were generated before entering
-  *		query_planner and then stashed in PlannerInfo.
-  */
- static void
- canonicalize_all_pathkeys(PlannerInfo *root)
- {
- 	root->query_pathkeys = canonicalize_pathkeys(root, root->query_pathkeys);
- 	root->group_pathkeys = canonicalize_pathkeys(root, root->group_pathkeys);
- 	root->window_pathkeys = canonicalize_pathkeys(root, root->window_pathkeys);
- 	root->distinct_pathkeys = canonicalize_pathkeys(root, root->distinct_pathkeys);
- 	root->sort_pathkeys = canonicalize_pathkeys(root, root->sort_pathkeys);
- }
--- 421,423 ----
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index db91b8277d9966d3597996a1f574233e6b062241..df274fe783081ab3771c0370c99387fe08a5dba2 100644
*** a/src/backend/optimizer/plan/planner.c
--- b/src/backend/optimizer/plan/planner.c
*************** planner_hook_type planner_hook = NULL;
*** 59,65 ****
--- 59,72 ----
  #define EXPRKIND_APPINFO		7
  #define EXPRKIND_PHV			8
  
+ /* Passthrough data for standard_qp_callback */
+ typedef struct
+ {
+ 	List	   *tlist;			/* preprocessed query targetlist */
+ 	List	   *activeWindows;	/* active windows, if any */
+ } standard_qp_extra;
  
+ /* Local functions */
  static Node *preprocess_expression(PlannerInfo *root, Node *expr, int kind);
  static void preprocess_qual_conditions(PlannerInfo *root, Node *jtnode);
  static Plan *inheritance_planner(PlannerInfo *root);
*************** static double preprocess_limit(PlannerIn
*** 70,75 ****
--- 77,83 ----
  				 int64 *offset_est, int64 *count_est);
  static bool limit_needed(Query *parse);
  static void preprocess_groupclause(PlannerInfo *root);
+ static void standard_qp_callback(PlannerInfo *root, void *extra);
  static bool choose_hashed_grouping(PlannerInfo *root,
  					   double tuple_fraction, double limit_tuples,
  					   double path_rows, int path_width,
*************** static List *select_active_windows(Plann
*** 94,100 ****
  static List *make_windowInputTargetList(PlannerInfo *root,
  						   List *tlist, List *activeWindows);
  static List *make_pathkeys_for_window(PlannerInfo *root, WindowClause *wc,
! 						 List *tlist, bool canonicalize);
  static void get_column_info_for_window(PlannerInfo *root, WindowClause *wc,
  						   List *tlist,
  						   int numSortCols, AttrNumber *sortColIdx,
--- 102,108 ----
  static List *make_windowInputTargetList(PlannerInfo *root,
  						   List *tlist, List *activeWindows);
  static List *make_pathkeys_for_window(PlannerInfo *root, WindowClause *wc,
! 						 List *tlist);
  static void get_column_info_for_window(PlannerInfo *root, WindowClause *wc,
  						   List *tlist,
  						   int numSortCols, AttrNumber *sortColIdx,
*************** grouping_planner(PlannerInfo *root, doub
*** 1052,1059 ****
  		 */
  		current_pathkeys = make_pathkeys_for_sortclauses(root,
  														 set_sortclauses,
! 													 result_plan->targetlist,
! 														 true);
  
  		/*
  		 * We should not need to call preprocess_targetlist, since we must be
--- 1060,1066 ----
  		 */
  		current_pathkeys = make_pathkeys_for_sortclauses(root,
  														 set_sortclauses,
! 													 result_plan->targetlist);
  
  		/*
  		 * We should not need to call preprocess_targetlist, since we must be
*************** grouping_planner(PlannerInfo *root, doub
*** 1082,1089 ****
  		Assert(parse->distinctClause == NIL);
  		root->sort_pathkeys = make_pathkeys_for_sortclauses(root,
  															parse->sortClause,
! 															tlist,
! 															true);
  	}
  	else
  	{
--- 1089,1095 ----
  		Assert(parse->distinctClause == NIL);
  		root->sort_pathkeys = make_pathkeys_for_sortclauses(root,
  															parse->sortClause,
! 															tlist);
  	}
  	else
  	{
*************** grouping_planner(PlannerInfo *root, doub
*** 1092,1097 ****
--- 1098,1104 ----
  		double		sub_limit_tuples;
  		AttrNumber *groupColIdx = NULL;
  		bool		need_tlist_eval = true;
+ 		standard_qp_extra qp_extra;
  		Path	   *cheapest_path;
  		Path	   *sorted_path;
  		Path	   *best_path;
*************** grouping_planner(PlannerInfo *root, doub
*** 1168,1249 ****
  		}
  
  		/*
- 		 * Calculate pathkeys that represent grouping/ordering requirements.
- 		 * Stash them in PlannerInfo so that query_planner can canonicalize
- 		 * them after EquivalenceClasses have been formed.	The sortClause is
- 		 * certainly sort-able, but GROUP BY and DISTINCT might not be, in
- 		 * which case we just leave their pathkeys empty.
- 		 */
- 		if (parse->groupClause &&
- 			grouping_is_sortable(parse->groupClause))
- 			root->group_pathkeys =
- 				make_pathkeys_for_sortclauses(root,
- 											  parse->groupClause,
- 											  tlist,
- 											  false);
- 		else
- 			root->group_pathkeys = NIL;
- 
- 		/* We consider only the first (bottom) window in pathkeys logic */
- 		if (activeWindows != NIL)
- 		{
- 			WindowClause *wc = (WindowClause *) linitial(activeWindows);
- 
- 			root->window_pathkeys = make_pathkeys_for_window(root,
- 															 wc,
- 															 tlist,
- 															 false);
- 		}
- 		else
- 			root->window_pathkeys = NIL;
- 
- 		if (parse->distinctClause &&
- 			grouping_is_sortable(parse->distinctClause))
- 			root->distinct_pathkeys =
- 				make_pathkeys_for_sortclauses(root,
- 											  parse->distinctClause,
- 											  tlist,
- 											  false);
- 		else
- 			root->distinct_pathkeys = NIL;
- 
- 		root->sort_pathkeys =
- 			make_pathkeys_for_sortclauses(root,
- 										  parse->sortClause,
- 										  tlist,
- 										  false);
- 
- 		/*
- 		 * Figure out whether we want a sorted result from query_planner.
- 		 *
- 		 * If we have a sortable GROUP BY clause, then we want a result sorted
- 		 * properly for grouping.  Otherwise, if we have window functions to
- 		 * evaluate, we try to sort for the first window.  Otherwise, if
- 		 * there's a sortable DISTINCT clause that's more rigorous than the
- 		 * ORDER BY clause, we try to produce output that's sufficiently well
- 		 * sorted for the DISTINCT.  Otherwise, if there is an ORDER BY
- 		 * clause, we want to sort by the ORDER BY clause.
- 		 *
- 		 * Note: if we have both ORDER BY and GROUP BY, and ORDER BY is a
- 		 * superset of GROUP BY, it would be tempting to request sort by ORDER
- 		 * BY --- but that might just leave us failing to exploit an available
- 		 * sort order at all.  Needs more thought.	The choice for DISTINCT
- 		 * versus ORDER BY is much easier, since we know that the parser
- 		 * ensured that one is a superset of the other.
- 		 */
- 		if (root->group_pathkeys)
- 			root->query_pathkeys = root->group_pathkeys;
- 		else if (root->window_pathkeys)
- 			root->query_pathkeys = root->window_pathkeys;
- 		else if (list_length(root->distinct_pathkeys) >
- 				 list_length(root->sort_pathkeys))
- 			root->query_pathkeys = root->distinct_pathkeys;
- 		else if (root->sort_pathkeys)
- 			root->query_pathkeys = root->sort_pathkeys;
- 		else
- 			root->query_pathkeys = NIL;
- 
- 		/*
  		 * Figure out whether there's a hard limit on the number of rows that
  		 * query_planner's result subplan needs to return.  Even if we know a
  		 * hard limit overall, it doesn't apply if the query has any
--- 1175,1180 ----
*************** grouping_planner(PlannerInfo *root, doub
*** 1258,1270 ****
  		else
  			sub_limit_tuples = limit_tuples;
  
  		/*
  		 * Generate the best unsorted and presorted paths for this Query (but
! 		 * note there may not be any presorted path).  query_planner will also
! 		 * estimate the number of groups in the query, and canonicalize all
! 		 * the pathkeys.
  		 */
  		query_planner(root, sub_tlist, tuple_fraction, sub_limit_tuples,
  					  &cheapest_path, &sorted_path, &dNumGroups);
  
  		/*
--- 1189,1207 ----
  		else
  			sub_limit_tuples = limit_tuples;
  
+ 		/* Set up data needed by standard_qp_callback */
+ 		qp_extra.tlist = tlist;
+ 		qp_extra.activeWindows = activeWindows;
+ 
  		/*
  		 * Generate the best unsorted and presorted paths for this Query (but
! 		 * note there may not be any presorted path).  We also generate (in
! 		 * standard_qp_callback) pathkey representations of the query's sort
! 		 * clause, distinct clause, etc.  query_planner will also estimate the
! 		 * number of groups in the query.
  		 */
  		query_planner(root, sub_tlist, tuple_fraction, sub_limit_tuples,
+ 					  standard_qp_callback, &qp_extra,
  					  &cheapest_path, &sorted_path, &dNumGroups);
  
  		/*
*************** grouping_planner(PlannerInfo *root, doub
*** 1597,1604 ****
  
  				window_pathkeys = make_pathkeys_for_window(root,
  														   wc,
! 														   tlist,
! 														   true);
  
  				/*
  				 * This is a bit tricky: we build a sort node even if we don't
--- 1534,1540 ----
  
  				window_pathkeys = make_pathkeys_for_window(root,
  														   wc,
! 														   tlist);
  
  				/*
  				 * This is a bit tricky: we build a sort node even if we don't
*************** preprocess_groupclause(PlannerInfo *root
*** 2440,2445 ****
--- 2376,2463 ----
  }
  
  /*
+  * Compute query_pathkeys and other pathkeys during plan generation
+  */
+ static void
+ standard_qp_callback(PlannerInfo *root, void *extra)
+ {
+ 	Query	   *parse = root->parse;
+ 	standard_qp_extra *qp_extra = (standard_qp_extra *) extra;
+ 	List	   *tlist = qp_extra->tlist;
+ 	List	   *activeWindows = qp_extra->activeWindows;
+ 
+ 	/*
+ 	 * Calculate pathkeys that represent grouping/ordering requirements.  The
+ 	 * sortClause is certainly sort-able, but GROUP BY and DISTINCT might not
+ 	 * be, in which case we just leave their pathkeys empty.
+ 	 */
+ 	if (parse->groupClause &&
+ 		grouping_is_sortable(parse->groupClause))
+ 		root->group_pathkeys =
+ 			make_pathkeys_for_sortclauses(root,
+ 										  parse->groupClause,
+ 										  tlist);
+ 	else
+ 		root->group_pathkeys = NIL;
+ 
+ 	/* We consider only the first (bottom) window in pathkeys logic */
+ 	if (activeWindows != NIL)
+ 	{
+ 		WindowClause *wc = (WindowClause *) linitial(activeWindows);
+ 
+ 		root->window_pathkeys = make_pathkeys_for_window(root,
+ 														 wc,
+ 														 tlist);
+ 	}
+ 	else
+ 		root->window_pathkeys = NIL;
+ 
+ 	if (parse->distinctClause &&
+ 		grouping_is_sortable(parse->distinctClause))
+ 		root->distinct_pathkeys =
+ 			make_pathkeys_for_sortclauses(root,
+ 										  parse->distinctClause,
+ 										  tlist);
+ 	else
+ 		root->distinct_pathkeys = NIL;
+ 
+ 	root->sort_pathkeys =
+ 		make_pathkeys_for_sortclauses(root,
+ 									  parse->sortClause,
+ 									  tlist);
+ 
+ 	/*
+ 	 * Figure out whether we want a sorted result from query_planner.
+ 	 *
+ 	 * If we have a sortable GROUP BY clause, then we want a result sorted
+ 	 * properly for grouping.  Otherwise, if we have window functions to
+ 	 * evaluate, we try to sort for the first window.  Otherwise, if there's a
+ 	 * sortable DISTINCT clause that's more rigorous than the ORDER BY clause,
+ 	 * we try to produce output that's sufficiently well sorted for the
+ 	 * DISTINCT.  Otherwise, if there is an ORDER BY clause, we want to sort
+ 	 * by the ORDER BY clause.
+ 	 *
+ 	 * Note: if we have both ORDER BY and GROUP BY, and ORDER BY is a superset
+ 	 * of GROUP BY, it would be tempting to request sort by ORDER BY --- but
+ 	 * that might just leave us failing to exploit an available sort order at
+ 	 * all.  Needs more thought.  The choice for DISTINCT versus ORDER BY is
+ 	 * much easier, since we know that the parser ensured that one is a
+ 	 * superset of the other.
+ 	 */
+ 	if (root->group_pathkeys)
+ 		root->query_pathkeys = root->group_pathkeys;
+ 	else if (root->window_pathkeys)
+ 		root->query_pathkeys = root->window_pathkeys;
+ 	else if (list_length(root->distinct_pathkeys) >
+ 			 list_length(root->sort_pathkeys))
+ 		root->query_pathkeys = root->distinct_pathkeys;
+ 	else if (root->sort_pathkeys)
+ 		root->query_pathkeys = root->sort_pathkeys;
+ 	else
+ 		root->query_pathkeys = NIL;
+ }
+ 
+ /*
   * choose_hashed_grouping - should we use hashed grouping?
   *
   * Returns TRUE to select hashing, FALSE to select sorting.
*************** make_windowInputTargetList(PlannerInfo *
*** 3235,3241 ****
   */
  static List *
  make_pathkeys_for_window(PlannerInfo *root, WindowClause *wc,
! 						 List *tlist, bool canonicalize)
  {
  	List	   *window_pathkeys;
  	List	   *window_sortclauses;
--- 3253,3259 ----
   */
  static List *
  make_pathkeys_for_window(PlannerInfo *root, WindowClause *wc,
! 						 List *tlist)
  {
  	List	   *window_pathkeys;
  	List	   *window_sortclauses;
*************** make_pathkeys_for_window(PlannerInfo *ro
*** 3257,3264 ****
  									 list_copy(wc->orderClause));
  	window_pathkeys = make_pathkeys_for_sortclauses(root,
  													window_sortclauses,
! 													tlist,
! 													canonicalize);
  	list_free(window_sortclauses);
  	return window_pathkeys;
  }
--- 3275,3281 ----
  									 list_copy(wc->orderClause));
  	window_pathkeys = make_pathkeys_for_sortclauses(root,
  													window_sortclauses,
! 													tlist);
  	list_free(window_sortclauses);
  	return window_pathkeys;
  }
*************** get_column_info_for_window(PlannerInfo *
*** 3336,3343 ****
  			sortclauses = lappend(sortclauses, sgc);
  			new_pathkeys = make_pathkeys_for_sortclauses(root,
  														 sortclauses,
! 														 tlist,
! 														 true);
  			if (list_length(new_pathkeys) > list_length(pathkeys))
  			{
  				/* this sort clause is actually significant */
--- 3353,3359 ----
  			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 */
*************** get_column_info_for_window(PlannerInfo *
*** 3355,3362 ****
  			sortclauses = lappend(sortclauses, sgc);
  			new_pathkeys = make_pathkeys_for_sortclauses(root,
  														 sortclauses,
! 														 tlist,
! 														 true);
  			if (list_length(new_pathkeys) > list_length(pathkeys))
  			{
  				/* this sort clause is actually significant */
--- 3371,3377 ----
  			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 */
diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h
index f34f6d5a991ccba12727294ff6e707b8d37f456a..15407e6a4ba021b64647a49d9f43cdab2fced9f2 100644
*** a/src/include/nodes/relation.h
--- b/src/include/nodes/relation.h
*************** typedef struct PlannerInfo
*** 206,212 ****
  	List	   *placeholder_list;		/* list of PlaceHolderInfos */
  
  	List	   *query_pathkeys; /* desired pathkeys for query_planner(), and
! 								 * actual pathkeys afterwards */
  
  	List	   *group_pathkeys; /* groupClause pathkeys, if any */
  	List	   *window_pathkeys;	/* pathkeys of bottom window, if any */
--- 206,212 ----
  	List	   *placeholder_list;		/* list of PlaceHolderInfos */
  
  	List	   *query_pathkeys; /* desired pathkeys for query_planner(), and
! 								 * actual pathkeys after planning */
  
  	List	   *group_pathkeys; /* groupClause pathkeys, if any */
  	List	   *window_pathkeys;	/* pathkeys of bottom window, if any */
diff --git a/src/include/optimizer/paths.h b/src/include/optimizer/paths.h
index 88ab4630fea850debfb9f250ac6efef2c818df41..9ef93c70c6412c5fd1a6171519e0715c0bab1002 100644
*** a/src/include/optimizer/paths.h
--- b/src/include/optimizer/paths.h
*************** typedef enum
*** 154,160 ****
  	PATHKEYS_DIFFERENT			/* neither pathkey includes the other */
  } PathKeysComparison;
  
- extern List *canonicalize_pathkeys(PlannerInfo *root, List *pathkeys);
  extern PathKeysComparison compare_pathkeys(List *keys1, List *keys2);
  extern bool pathkeys_contained_in(List *keys1, List *keys2);
  extern Path *get_cheapest_path_for_pathkeys(List *paths, List *pathkeys,
--- 154,159 ----
*************** extern List *build_join_pathkeys(Planner
*** 174,181 ****
  					List *outer_pathkeys);
  extern List *make_pathkeys_for_sortclauses(PlannerInfo *root,
  							  List *sortclauses,
! 							  List *tlist,
! 							  bool canonicalize);
  extern void initialize_mergeclause_eclasses(PlannerInfo *root,
  								RestrictInfo *restrictinfo);
  extern void update_mergeclause_eclasses(PlannerInfo *root,
--- 173,179 ----
  					List *outer_pathkeys);
  extern List *make_pathkeys_for_sortclauses(PlannerInfo *root,
  							  List *sortclauses,
! 							  List *tlist);
  extern void initialize_mergeclause_eclasses(PlannerInfo *root,
  								RestrictInfo *restrictinfo);
  extern void update_mergeclause_eclasses(PlannerInfo *root,
diff --git a/src/include/optimizer/planmain.h b/src/include/optimizer/planmain.h
index 16d685846e8655a2b7726ed04c47110e87602f28..33eaf32627225f23d1a8ce1aa73c3848bc3be7aa 100644
*** a/src/include/optimizer/planmain.h
--- b/src/include/optimizer/planmain.h
***************
*** 21,31 ****
--- 21,35 ----
  #define DEFAULT_CURSOR_TUPLE_FRACTION 0.1
  extern double cursor_tuple_fraction;
  
+ /* query_planner callback to compute query_pathkeys */
+ typedef void (*query_pathkeys_callback) (PlannerInfo *root, void *extra);
+ 
  /*
   * prototypes for plan/planmain.c
   */
  extern void query_planner(PlannerInfo *root, List *tlist,
  			  double tuple_fraction, double limit_tuples,
+ 			  query_pathkeys_callback qp_callback, void *qp_extra,
  			  Path **cheapest_path, Path **sorted_path,
  			  double *num_groups);
  
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 3421a559f25e2d59a7e715db28a1bff03a81af4c..31c2a320a6d1aa47ef355c6901266d7a1aeb518b 100644
*** a/src/test/regress/expected/join.out
--- b/src/test/regress/expected/join.out
*************** select b.unique1 from
*** 2826,2831 ****
--- 2826,2860 ----
          
  (5 rows)
  
+ explain (costs off)
+ select * from
+ (
+   select unique1, q1, coalesce(unique1, -1) + q1 as fault
+   from int8_tbl left join tenk1 on (q2 = unique2)
+ ) ss
+ where fault = 122
+ order by fault;
+                            QUERY PLAN                            
+ -----------------------------------------------------------------
+  Nested Loop Left Join
+    Filter: ((COALESCE(tenk1.unique1, (-1)) + int8_tbl.q1) = 122)
+    ->  Seq Scan on int8_tbl
+    ->  Index Scan using tenk1_unique2 on tenk1
+          Index Cond: (int8_tbl.q2 = unique2)
+ (5 rows)
+ 
+ select * from
+ (
+   select unique1, q1, coalesce(unique1, -1) + q1 as fault
+   from int8_tbl left join tenk1 on (q2 = unique2)
+ ) ss
+ where fault = 122
+ order by fault;
+  unique1 | q1  | fault 
+ ---------+-----+-------
+          | 123 |   122
+ (1 row)
+ 
  --
  -- test handling of potential equivalence clauses above outer joins
  --
diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index 6f51b8532763eb2bddde7872be2ce2874e51240b..656766acd3fbb499ef41da99e63bc79fcea0b108 100644
*** a/src/test/regress/sql/join.sql
--- b/src/test/regress/sql/join.sql
*************** select b.unique1 from
*** 749,754 ****
--- 749,771 ----
    right join int4_tbl i2 on i2.f1 = b.tenthous
    order by 1;
  
+ explain (costs off)
+ select * from
+ (
+   select unique1, q1, coalesce(unique1, -1) + q1 as fault
+   from int8_tbl left join tenk1 on (q2 = unique2)
+ ) ss
+ where fault = 122
+ order by fault;
+ 
+ select * from
+ (
+   select unique1, q1, coalesce(unique1, -1) + q1 as fault
+   from int8_tbl left join tenk1 on (q2 = unique2)
+ ) ss
+ where fault = 122
+ order by fault;
+ 
  --
  -- test handling of potential equivalence clauses above outer joins
  --
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to