indxpath.c's references to IndexOptInfo.ncolumns are all wrong, no?

Поиск
Список
Период
Сортировка
От Tom Lane
Тема indxpath.c's references to IndexOptInfo.ncolumns are all wrong, no?
Дата
Msg-id 25526.1549847928@sss.pgh.pa.us
обсуждение исходный текст
Ответы Re: indxpath.c's references to IndexOptInfo.ncolumns are all wrong, no?  (Peter Geoghegan <pg@bowt.ie>)
Список pgsql-hackers
Apparently, whoever went through indxpath.c to substitute nkeycolumns
for ncolumns was not paying attention.  As far as I can tell, the
*only* place in there where it's correct to reference ncolumns is in
check_index_only, where we determine which columns can be extracted from
an index-only scan.  A lot of the other places are obviously wrong, eg in
relation_has_unique_index_for:

        for (c = 0; c < ind->ncolumns; c++)
        ...
                if (!list_member_oid(rinfo->mergeopfamilies, ind->opfamily[c]))

Even if it were plausible that an INCLUDE column is something to consider
when deciding whether the index enforces uniqueness, this code accesses
beyond the documented end of the opfamily[] array :-(

The fact that the regression tests haven't caught this doesn't give
me a warm feeling about how thoroughly the included-columns logic has
been tested.  It's really easy to make it fall over, for instance

regression=# explain select * from tenk1 where (thousand, tenthous) < (10,100);
                                     QUERY PLAN

--------------------------------------------------------------------------------
-----
 Bitmap Heap Scan on tenk1  (cost=5.11..233.86 rows=107 width=244)
   Recheck Cond: (ROW(thousand, tenthous) < ROW(10, 100))
   ->  Bitmap Index Scan on tenk1_thous_tenthous  (cost=0.00..5.09 rows=107 widt
h=0)
         Index Cond: (ROW(thousand, tenthous) < ROW(10, 100))
(4 rows)

regression=# drop index tenk1_thous_tenthous;
DROP INDEX
regression=# create index on tenk1(thousand) include (tenthous);
CREATE INDEX
regression=# explain select * from tenk1 where (thousand, tenthous) < (10,100);
ERROR:  operator 97 is not a member of opfamily 2139062142

I've got mixed feelings about whether to try to fix this before
tomorrow's wraps.  The attached patch seems correct and passes
check-world, but there's sure not a lot of margin for error now.
Thoughts?

            regards, tom lane

diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index 51d2da5..a55b9e0 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -258,7 +258,7 @@ create_index_paths(PlannerInfo *root, RelOptInfo *rel)
         IndexOptInfo *index = (IndexOptInfo *) lfirst(lc);

         /* Protect limited-size array in IndexClauseSets */
-        Assert(index->ncolumns <= INDEX_MAX_KEYS);
+        Assert(index->nkeycolumns <= INDEX_MAX_KEYS);

         /*
          * Ignore partial indexes that do not match the query.
@@ -473,7 +473,7 @@ consider_index_join_clauses(PlannerInfo *root, RelOptInfo *rel,
      * relation itself is also included in the relids set.  considered_relids
      * lists all relids sets we've already tried.
      */
-    for (indexcol = 0; indexcol < index->ncolumns; indexcol++)
+    for (indexcol = 0; indexcol < index->nkeycolumns; indexcol++)
     {
         /* Consider each applicable simple join clause */
         considered_clauses += list_length(jclauseset->indexclauses[indexcol]);
@@ -628,7 +628,7 @@ get_join_index_paths(PlannerInfo *root, RelOptInfo *rel,
     /* Identify indexclauses usable with this relids set */
     MemSet(&clauseset, 0, sizeof(clauseset));

-    for (indexcol = 0; indexcol < index->ncolumns; indexcol++)
+    for (indexcol = 0; indexcol < index->nkeycolumns; indexcol++)
     {
         ListCell   *lc;

@@ -925,7 +925,7 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel,
     index_clauses = NIL;
     found_lower_saop_clause = false;
     outer_relids = bms_copy(rel->lateral_relids);
-    for (indexcol = 0; indexcol < index->ncolumns; indexcol++)
+    for (indexcol = 0; indexcol < index->nkeycolumns; indexcol++)
     {
         ListCell   *lc;

@@ -2680,7 +2680,7 @@ match_pathkeys_to_index(IndexOptInfo *index, List *pathkeys,
              * amcanorderbyop.  We might need different logic in future for
              * other implementations.
              */
-            for (indexcol = 0; indexcol < index->ncolumns; indexcol++)
+            for (indexcol = 0; indexcol < index->nkeycolumns; indexcol++)
             {
                 Expr       *expr;

@@ -3111,7 +3111,7 @@ relation_has_unique_index_for(PlannerInfo *root, RelOptInfo *rel,
          * Try to find each index column in the lists of conditions.  This is
          * O(N^2) or worse, but we expect all the lists to be short.
          */
-        for (c = 0; c < ind->ncolumns; c++)
+        for (c = 0; c < ind->nkeycolumns; c++)
         {
             bool        matched = false;
             ListCell   *lc;
@@ -3187,7 +3187,7 @@ relation_has_unique_index_for(PlannerInfo *root, RelOptInfo *rel,
         }

         /* Matched all columns of this index? */
-        if (c == ind->ncolumns)
+        if (c == ind->nkeycolumns)
             return true;
     }

@@ -3991,7 +3991,7 @@ expand_indexqual_rowcompare(RestrictInfo *rinfo,

                 break;
         }
-        if (i >= index->ncolumns)
+        if (i >= index->nkeycolumns)
             break;                /* no match found */

         /* Add column number to returned list */

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Thomas Munro
Дата:
Сообщение: Re: dsa_allocate() faliure
Следующее
От: Tom Lane
Дата:
Сообщение: Re: dsa_allocate() faliure