Content-Type: multipart/mixed; boundary="------------e7BiyXuZvHaYrEu1cEFykVFE"
Message-ID: <bf3a6ea1-a7d8-7211-0669-189d5c169374@enterprisedb.com>
Date: Fri, 14 Jan 2022 01:48:33 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.2.0
Content-Language: en-US
To: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
From: Tomas Vondra <tomas.vondra@enterprisedb.com>
Subject: minor bug in sort_inner_and_outer()
Archived-At: 
 <https://www.postgresql.org/message-id/bf3a6ea1-a7d8-7211-0669-189d5c169374%40enterprisedb.com>
Precedence: bulk

This is a multi-part message in MIME format.
--------------e7BiyXuZvHaYrEu1cEFykVFE
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit

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
--------------e7BiyXuZvHaYrEu1cEFykVFE
Content-Type: text/x-patch; charset=UTF-8;
 name="sort-inner-and-outer-fix.patch"
Content-Disposition: attachment; filename="sort-inner-and-outer-fix.patch"
Content-Transfer-Encoding: base64

ZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL29wdGltaXplci9wYXRoL2pvaW5wYXRoLmMgYi9z
cmMvYmFja2VuZC9vcHRpbWl6ZXIvcGF0aC9qb2lucGF0aC5jCmluZGV4IGY5NmZjOWZkMjgy
Li44MWUwNGI2YWM5ZCAxMDA2NDQKLS0tIGEvc3JjL2JhY2tlbmQvb3B0aW1pemVyL3BhdGgv
am9pbnBhdGguYworKysgYi9zcmMvYmFja2VuZC9vcHRpbWl6ZXIvcGF0aC9qb2lucGF0aC5j
CkBAIC0xMjU4LDcgKzEyNTgsNyBAQCBzb3J0X2lubmVyX2FuZF9vdXRlcihQbGFubmVySW5m
byAqcm9vdCwKIAogCWZvcmVhY2gobCwgYWxsX3BhdGhrZXlzKQogCXsKLQkJTGlzdAkgICAq
ZnJvbnRfcGF0aGtleSA9IChMaXN0ICopIGxmaXJzdChsKTsKKwkJUGF0aEtleQkgICAqZnJv
bnRfcGF0aGtleSA9IGxmaXJzdF9ub2RlKFBhdGhLZXksIGwpOwogCQlMaXN0CSAgICpjdXJf
bWVyZ2VjbGF1c2VzOwogCQlMaXN0CSAgICpvdXRlcmtleXM7CiAJCUxpc3QJICAgKmlubmVy
a2V5czsK
--------------e7BiyXuZvHaYrEu1cEFykVFE--