[Tomas Vondra <tomas.vondra@enterprisedb.com>]

Hi,

While looking at sort_inner_and_outer() I was rather confused what is
stored in all_pathkeys, because the code does this:

  List *all_pathkeys;

  ...

  all_pathkeys = select_outer_pathkeys_for_merge(root,
                                             extra->mergeclause_list,
                                             joinrel);

  foreach(l, all_pathkeys)
  {
      List	   *front_pathkey = (List *) lfirst(l);
      ...

      /* Make a pathkey list with this guy first */
      if (l != list_head(all_pathkeys))
          outerkeys = lcons(front_pathkey,
                            ...);
      else
          ...

which seems to suggest all_pathkeys is a list of lists, because why else
would front_pathkey be a (List *). But that doesn't seem to be the case,
front_pathkey is actually a PathKey, not a List, as demonstrated by gdb:

(gdb) p *front_pathkey
$2 = {type = T_PathKey, length = 0, ...}

Maybe it's some clever list-fu that I can't comprehend, but I guess it's
a bug present since ~2004. It's benign because we only ever pass the
front_pathkey to lcons() which does not really care.


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Attachments:

  [text/x-patch] sort-inner-and-outer-fix.patch (492B, ../bf3a6ea1-a7d8-7211-0669-189d5c169374@enterprisedb.com/2-sort-inner-and-outer-fix.patch)
  download | inline diff:
diff --git a/src/backend/optimizer/path/joinpath.c b/src/backend/optimizer/path/joinpath.c
index f96fc9fd282..81e04b6ac9d 100644
--- a/src/backend/optimizer/path/joinpath.c
+++ b/src/backend/optimizer/path/joinpath.c
@@ -1258,7 +1258,7 @@ sort_inner_and_outer(PlannerInfo *root,
 
 	foreach(l, all_pathkeys)
 	{
-		List	   *front_pathkey = (List *) lfirst(l);
+		PathKey	   *front_pathkey = lfirst_node(PathKey, l);
 		List	   *cur_mergeclauses;
 		List	   *outerkeys;
 		List	   *innerkeys;