Skip to content

Commit ac4913a

Browse files
tglsfdctglsfdc
committed
Clean up messy clause-selectivity code in clausesel.c; repair bug
identified by Hiroshi (incorrect cost attributed to OR clauses after multiple passes through set_rest_selec()). I think the code was trying to allow selectivities of OR subclauses to be passed in from outside, but noplace was actually passing any useful data, and set_rest_selec() was passing wrong data. Restructure representation of "indexqual" in IndexPath nodes so that it is the same as for indxqual in completed IndexScan nodes: namely, a toplevel list with an entry for each pass of the index scan, having sublists that are implicitly-ANDed index qual conditions for that pass. You don't want to know what the old representation was :-( Improve documentation of OR-clause indexscan functions. Remove useless 'notclause' field from RestrictInfo nodes. (This might force an initdb for anyone who has stored rules containing RestrictInfos, but I do not think that RestrictInfo ever appears in completed plans.)
1 parent 348bdbc commit ac4913a

File tree

17 files changed

+471
-519
lines changed

17 files changed

+471
-519
lines changed

‎src/backend/nodes/copyfuncs.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
*
99
* IDENTIFICATION
10-
* $Header: /cvsroot/pgsql/src/backend/nodes/copyfuncs.c,v 1.86 1999/07/17 20:17:05 momjian Exp $
10+
* $Header: /cvsroot/pgsql/src/backend/nodes/copyfuncs.c,v 1.87 1999/07/24 23:21:06 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -1317,7 +1317,6 @@ _copyRestrictInfo(RestrictInfo *from)
13171317
Node_Copy(from, newnode, clause);
13181318

13191319
newnode->selectivity = from->selectivity;
1320-
newnode->notclause = from->notclause;
13211320

13221321
Node_Copy(from, newnode, indexids);
13231322
Node_Copy(from, newnode, mergejoinorder);

‎src/backend/nodes/equalfuncs.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
*
99
* IDENTIFICATION
10-
* $Header: /cvsroot/pgsql/src/backend/nodes/equalfuncs.c,v 1.43 1999/07/17 20:17:05 momjian Exp $
10+
* $Header: /cvsroot/pgsql/src/backend/nodes/equalfuncs.c,v 1.44 1999/07/24 23:21:06 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -305,8 +305,6 @@ _equalRestrictInfo(RestrictInfo *a, RestrictInfo *b)
305305
return false;
306306
if (a->selectivity != b->selectivity)
307307
return false;
308-
if (a->notclause != b->notclause)
309-
return false;
310308
#ifdef EqualMergeOrderExists
311309
if (!EqualMergeOrder(a->mergejoinorder, b->mergejoinorder))
312310
return false;

‎src/backend/nodes/outfuncs.c

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
*
66
* Copyright (c) 1994, Regents of the University of California
77
*
8-
* $Id: outfuncs.c,v 1.90 1999/07/18 19:02:49 tgl Exp $
8+
* $Id: outfuncs.c,v 1.91 1999/07/24 23:21:07 tgl Exp $
99
*
1010
* NOTES
1111
* Every (plan) node in POSTGRES has an associated "out" routine which
@@ -1110,9 +1110,8 @@ _outRestrictInfo(StringInfo str, RestrictInfo *node)
11101110
_outNode(str, node->clause);
11111111

11121112
appendStringInfo(str,
1113-
" :selectivity %f :notclause %s :indexids ",
1114-
node->selectivity,
1115-
node->notclause ? "true" : "false");
1113+
" :selectivity %f :indexids ",
1114+
node->selectivity);
11161115
_outNode(str, node->indexids);
11171116

11181117
appendStringInfo(str, " :mergejoinorder ");

‎src/backend/nodes/readfuncs.c

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
*
99
* IDENTIFICATION
10-
* $Header: /cvsroot/pgsql/src/backend/nodes/readfuncs.c,v 1.69 1999/07/17 20:17:08 momjian Exp $
10+
* $Header: /cvsroot/pgsql/src/backend/nodes/readfuncs.c,v 1.70 1999/07/24 23:21:08 tgl Exp $
1111
*
1212
* NOTES
1313
* Most of the read functions for plan nodes are tested. (In fact, they
@@ -1856,14 +1856,6 @@ _readRestrictInfo()
18561856

18571857
local_node->selectivity = atof(token);
18581858

1859-
token = lsptok(NULL, &length); /* get :notclause */
1860-
token = lsptok(NULL, &length); /* now read it */
1861-
1862-
if (!strncmp(token, "true", 4))
1863-
local_node->notclause = true;
1864-
else
1865-
local_node->notclause = false;
1866-
18671859
token = lsptok(NULL, &length); /* get :indexids */
18681860
local_node->indexids = nodeRead(true); /* now read it */
18691861

‎src/backend/optimizer/path/allpaths.c

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
*
99
* IDENTIFICATION
10-
* $Header: /cvsroot/pgsql/src/backend/optimizer/path/allpaths.c,v 1.50 1999/07/17 20:17:11 momjian Exp $
10+
* $Header: /cvsroot/pgsql/src/backend/optimizer/path/allpaths.c,v 1.51 1999/07/24 23:21:08 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -86,8 +86,8 @@ make_one_rel(Query *root, List *rels)
8686
* set_base_rel_pathlist
8787
* Finds all paths available for scanning each relation entry in
8888
* 'rels'. Sequential scan and any available indices are considered
89-
* if possible(indices are not considered for lower nesting levels).
90-
* All unique paths are attached to the relation's 'pathlist' field.
89+
* if possible (indices are not considered for lower nesting levels).
90+
* All useful paths are attached to the relation's 'pathlist' field.
9191
*
9292
* MODIFIES: rels
9393
*/
@@ -98,21 +98,32 @@ set_base_rel_pathlist(Query *root, List *rels)
9898

9999
foreach(temp, rels)
100100
{
101+
RelOptInfo *rel = (RelOptInfo *) lfirst(temp);
102+
List *indices = find_relation_indices(root, rel);
101103
List *sequential_scan_list;
102104
List *rel_index_scan_list;
103105
List *or_index_scan_list;
104-
RelOptInfo *rel = (RelOptInfo *) lfirst(temp);
105106

106107
sequential_scan_list = lcons(create_seqscan_path(rel), NIL);
107108

108109
rel_index_scan_list = create_index_paths(root,
109110
rel,
110-
find_relation_indices(root, rel),
111+
indices,
111112
rel->restrictinfo,
112113
rel->joininfo);
113114

114-
or_index_scan_list = create_or_index_paths(root, rel, rel->restrictinfo);
115+
/* Note: create_or_index_paths depends on create_index_paths
116+
* to have marked OR restriction clauses with relevant indices;
117+
* this is why it doesn't need to be given the full list of indices.
118+
*/
115119

120+
or_index_scan_list = create_or_index_paths(root, rel,
121+
rel->restrictinfo);
122+
123+
/* add_pathlist will discard any paths that are dominated by
124+
* another available path, keeping only those paths that are
125+
* superior along at least one dimension of cost or sortedness.
126+
*/
116127
rel->pathlist = add_pathlist(rel,
117128
sequential_scan_list,
118129
nconc(rel_index_scan_list,
@@ -128,7 +139,6 @@ set_base_rel_pathlist(Query *root, List *rels)
128139
rel->size = compute_rel_size(rel);
129140
rel->width = compute_rel_width(rel);
130141
}
131-
return;
132142
}
133143

134144
/*

0 commit comments

Comments
 (0)