Fix additional breakage in covering-index patch.
authorTom Lane <tgl@sss.pgh.pa.us>
Sun, 8 Apr 2018 21:23:39 +0000 (17:23 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Sun, 8 Apr 2018 21:23:39 +0000 (17:23 -0400)
CheckIndexCompatible() misused ComputeIndexAttrs() by not bothering
to fill ii_NumIndexAttrs and ii_NumIndexKeyAttrs in the passed
IndexInfo.  Omission of ii_NumIndexAttrs was previously unimportant,
but now this matters because ComputeIndexAttrs depends on
ii_NumIndexKeyAttrs to decide how many columns it needs to report on.

(BTW, the fact that this oversight wasn't detected earlier implies
that we have no regression test verifying whether CheckIndexCompatible
ever succeeds.  Bad dog.  Not the job of this patch to fix it, though.)

Also, change the API of ComputeIndexAttrs so that it fills the opclass
output array for all column positions, as it does for the options output
array; positions for non-key index columns are filled with zeroes.
This isn't directly fixing any bug, but it seems like a good idea.

Per valgrind failure reports from buildfarm.

Alexander Korotkov, tweaked a bit by me

Discussion: https://postgr.es/m/CAPpHfduWrysrT-qAhn+3Ea5+Mg6Vhc-oA6o2Z-hRCPRdvf3tiw@mail.gmail.com

src/backend/commands/indexcmds.c

index 3fb2344198a77bd8436330bdeb9147816fde1eaf..860a60d1096d264e0d3534c50a063848e5bddc65 100644 (file)
@@ -182,9 +182,13 @@ CheckIndexCompatible(Oid oldId,
     * the new index, so we can test whether it's compatible with the existing
     * one.  Note that ComputeIndexAttrs might fail here, but that's OK:
     * DefineIndex would have called this function with the same arguments
-    * later on, and it would have failed then anyway.
+    * later on, and it would have failed then anyway.  Our attributeList
+    * contains only key attributes, thus we're filling ii_NumIndexAttrs and
+    * ii_NumIndexKeyAttrs with same value.
     */
    indexInfo = makeNode(IndexInfo);
+   indexInfo->ii_NumIndexAttrs = numberOfAttributes;
+   indexInfo->ii_NumIndexKeyAttrs = numberOfAttributes;
    indexInfo->ii_Expressions = NIL;
    indexInfo->ii_ExpressionsState = NIL;
    indexInfo->ii_PredicateState = NULL;
@@ -650,7 +654,7 @@ DefineIndex(Oid relationId,
 
    typeObjectId = (Oid *) palloc(numberOfAttributes * sizeof(Oid));
    collationObjectId = (Oid *) palloc(numberOfAttributes * sizeof(Oid));
-   classObjectId = (Oid *) palloc(numberOfKeyAttributes * sizeof(Oid));
+   classObjectId = (Oid *) palloc(numberOfAttributes * sizeof(Oid));
    coloptions = (int16 *) palloc(numberOfAttributes * sizeof(int16));
    ComputeIndexAttrs(indexInfo,
                      typeObjectId, collationObjectId, classObjectId,
@@ -1518,10 +1522,11 @@ ComputeIndexAttrs(IndexInfo *indexInfo,
        collationOidP[attn] = attcollation;
 
        /*
-        * Skip opclass and ordering options for included columns.
+        * Included columns have no opclass and no ordering options.
         */
        if (attn >= nkeycols)
        {
+           classOidP[attn] = InvalidOid;
            colOptionP[attn] = 0;
            attn++;
            continue;