Skip to content

Commit 91f82de

Browse files
tglsfdctglsfdc
committed
Assign sort keys properly when there are duplicate entries in
pathkey list --- corrects misbehavior seen with multiple mergejoin clauses mentioning same variable.
1 parent 5517938 commit 91f82de

File tree

1 file changed

+63
-35
lines changed

1 file changed

+63
-35
lines changed

‎src/backend/optimizer/plan/createplan.c

Lines changed: 63 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
*
99
* IDENTIFICATION
10-
* $Header: /cvsroot/pgsql/src/backend/optimizer/plan/createplan.c,v 1.71 1999/08/16 02:17:53 tgl Exp $
10+
* $Header: /cvsroot/pgsql/src/backend/optimizer/plan/createplan.c,v 1.72 1999/08/16 23:07:20 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -28,7 +28,7 @@
2828

2929

3030
static List *switch_outer(List *clauses);
31-
static List *set_tlist_sort_info(List *tlist, List *pathkeys);
31+
static int set_tlist_sort_info(List *tlist, List *pathkeys);
3232
static Scan *create_scan_node(Path *best_path, List *tlist);
3333
static Join *create_join_node(JoinPath *best_path, List *tlist);
3434
static SeqScan *create_seqscan_node(Path *best_path, List *tlist,
@@ -804,40 +804,65 @@ switch_outer(List *clauses)
804804
* Sets the reskey and reskeyop fields of resdom nodes in a target list
805805
* for a sort node.
806806
*
807-
* 'tlist' is the target list
807+
* 'tlist' is the target list (which is modified in-place).
808+
* tlist's reskey fields must be clear to start with.
808809
* 'pathkeys' is the desired pathkeys for the sort. NIL means no sort.
809810
*
810-
* Returns the modified-in-place target list.
811+
* Returns the number of sort keys assigned (which might be less than
812+
* length(pathkeys)!)
811813
*/
812-
static List *
814+
static int
813815
set_tlist_sort_info(List *tlist, List *pathkeys)
814816
{
815-
int keyno = 1;
817+
int keysassigned = 0;
816818
List *i;
817819

818820
foreach(i, pathkeys)
819821
{
820822
List *keysublist = (List *) lfirst(i);
821-
PathKeyItem *pathkey;
822-
Resdom *resdom;
823+
PathKeyItem *pathkey = NULL;
824+
Resdom *resdom = NULL;
825+
List *j;
823826

824827
/*
825828
* We can sort by any one of the sort key items listed in this
826-
* sublist. For now, we always take the first one --- is there
827-
* any way of figuring out which might be cheapest to execute?
828-
* (For example, int4lt is likely much cheaper to execute than
829-
* numericlt, but both might appear in the same pathkey sublist...)
829+
* sublist. For now, we take the first one that corresponds to
830+
* an available Var in the tlist.
831+
*
832+
* XXX if we have a choice, is there any way of figuring out which
833+
* might be cheapest to execute? (For example, int4lt is likely
834+
* much cheaper to execute than numericlt, but both might appear in
835+
* the same pathkey sublist...) Not clear that we ever will have
836+
* a choice in practice, so it may not matter.
830837
*/
831-
pathkey = lfirst(keysublist);
832-
Assert(IsA(pathkey, PathKeyItem));
833-
resdom = tlist_member((Var *) pathkey->key, tlist);
838+
foreach(j, keysublist)
839+
{
840+
pathkey = lfirst(j);
841+
Assert(IsA(pathkey, PathKeyItem));
842+
resdom = tlist_member((Var *) pathkey->key, tlist);
843+
if (resdom)
844+
break;
845+
}
834846
if (!resdom)
835847
elog(ERROR, "set_tlist_sort_info: cannot find tlist item to sort");
836-
resdom->reskey = keyno;
837-
resdom->reskeyop = get_opcode(pathkey->sortop);
838-
keyno++;
848+
849+
/*
850+
* The resdom might be already marked as a sort key, if the pathkeys
851+
* contain duplicate entries. (This can happen in scenarios where
852+
* multiple mergejoinable clauses mention the same var, for example.)
853+
* In that case the current pathkey is essentially a no-op, because
854+
* only one value can be seen within any subgroup where it would be
855+
* consulted. We can ignore it.
856+
*/
857+
if (resdom->reskey == 0)
858+
{
859+
/* OK, mark it as a sort key and set the sort operator regproc */
860+
resdom->reskey = ++keysassigned;
861+
resdom->reskeyop = get_opcode(pathkey->sortop);
862+
}
839863
}
840-
return tlist;
864+
865+
return keysassigned;
841866
}
842867

843868
/*
@@ -885,34 +910,37 @@ make_noname(List *tlist,
885910
Plan *plan_node)
886911
{
887912
List *noname_tlist;
913+
int numsortkeys;
914+
Plan *tmpplan;
888915
Noname *retval;
889916

890917
/* Create a new target list for the noname, with sort keys set. */
891-
noname_tlist = set_tlist_sort_info(new_unsorted_tlist(tlist),
892-
pathkeys);
918+
noname_tlist = new_unsorted_tlist(tlist);
919+
numsortkeys = set_tlist_sort_info(noname_tlist, pathkeys);
893920

894-
if (pathkeys != NIL)
921+
if (numsortkeys > 0)
895922
{
896923
/* need to sort */
897-
retval = (Noname *) make_seqscan(tlist,
898-
NIL,
899-
_NONAME_RELATION_ID_,
900-
(Plan *) make_sort(noname_tlist,
901-
_NONAME_RELATION_ID_,
902-
plan_node,
903-
length(pathkeys)));
924+
tmpplan = (Plan *) make_sort(noname_tlist,
925+
_NONAME_RELATION_ID_,
926+
plan_node,
927+
numsortkeys);
904928
}
905929
else
906930
{
907931
/* no sort */
908-
retval = (Noname *) make_seqscan(tlist,
909-
NIL,
932+
tmpplan = (Plan *) make_material(noname_tlist,
910933
_NONAME_RELATION_ID_,
911-
(Plan *) make_material(noname_tlist,
912-
_NONAME_RELATION_ID_,
913-
plan_node,
914-
0));
934+
plan_node,
935+
0);
915936
}
937+
938+
/* Return a seqscan using the original tlist */
939+
retval = (Noname *) make_seqscan(tlist,
940+
NIL,
941+
_NONAME_RELATION_ID_,
942+
tmpplan);
943+
916944
return retval;
917945
}
918946

0 commit comments

Comments
 (0)