Cleanup covering infrastructure

- Explicitly forbids opclass, collation and indoptions (like DESC/ASC etc) for
  including columns. Throw an error if user points that.
- Truncated storage arrays for such attributes to store only key atrributes,
  added assertion checks.
- Do not check opfamily and collation for including columns in
  CompareIndexInfo()

Discussion: https://www.postgresql.org/message-id/5ee72852-3c4e-ee35-e2ed-c1d053d45c08@sigaev.ru

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 7fc7c01d766c181ab242e15223135bb753f9c304..dec4265d68101abb76974cf62809c7adca684c92 100644 (file)
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -297,6 +297,7 @@ ConstructTupleDescriptor(Relation heapRelation,
                                                 Oid *classObjectId)
 {
        int                     numatts = indexInfo->ii_NumIndexAttrs;
+       int                     numkeyatts = indexInfo->ii_NumIndexKeyAttrs;
        ListCell   *colnames_item = list_head(indexColNames);
        ListCell   *indexpr_item = list_head(indexInfo->ii_Expressions);
        IndexAmRoutine *amroutine;
@@ -375,7 +376,8 @@ ConstructTupleDescriptor(Relation heapRelation,
                        to->attidentity = '\0';
                        to->attislocal = true;
                        to->attinhcount = 0;
-                       to->attcollation = collationObjectId[i];
+                       to->attcollation = (i < numkeyatts) ?
+                                                       collationObjectId[i] : InvalidOid;
                }
                else
                {
@@ -411,7 +413,8 @@ ConstructTupleDescriptor(Relation heapRelation,
                        to->attcacheoff = -1;
                        to->atttypmod = exprTypmod(indexkey);
                        to->attislocal = true;
-                       to->attcollation = collationObjectId[i];
+                       to->attcollation = (i < numkeyatts) ?
+                                                       collationObjectId[i] : InvalidOid;
 
                        ReleaseSysCache(tuple);
 
@@ -608,9 +611,9 @@ UpdateIndexRelation(Oid indexoid,
        indkey = buildint2vector(NULL, indexInfo->ii_NumIndexAttrs);
        for (i = 0; i < indexInfo->ii_NumIndexAttrs; i++)
                indkey->values[i] = indexInfo->ii_IndexAttrNumbers[i];
-       indcollation = buildoidvector(collationOids, indexInfo->ii_NumIndexAttrs);
+       indcollation = buildoidvector(collationOids, indexInfo->ii_NumIndexKeyAttrs);
        indclass = buildoidvector(classOids, indexInfo->ii_NumIndexKeyAttrs);
-       indoption = buildint2vector(coloptions, indexInfo->ii_NumIndexAttrs);
+       indoption = buildint2vector(coloptions, indexInfo->ii_NumIndexKeyAttrs);
 
        /*
         * Convert the index expressions (if any) to a text datum
@@ -1081,7 +1084,7 @@ index_create(Relation heapRelation,
 
                /* Store dependency on collations */
                /* The default collation is pinned, so don't bother recording it */
-               for (i = 0; i < indexInfo->ii_NumIndexAttrs; i++)
+               for (i = 0; i < indexInfo->ii_NumIndexKeyAttrs; i++)
                {
                        if (OidIsValid(collationObjectId[i]) &&
                                collationObjectId[i] != DEFAULT_COLLATION_OID)
@@ -1832,6 +1835,11 @@ CompareIndexInfo(IndexInfo *info1, IndexInfo *info2,
        if (info1->ii_NumIndexAttrs != info2->ii_NumIndexAttrs)
                return false;
 
+       /* and same number of key attributes */
+       if (info1->ii_NumIndexKeyAttrs != info2->ii_NumIndexKeyAttrs)
+               return false;
+
+
        /*
         * and columns match through the attribute map (actual attribute numbers
         * might differ!)  Note that this implies that index columns that are
@@ -1849,6 +1857,10 @@ CompareIndexInfo(IndexInfo *info1, IndexInfo *info2,
                        info1->ii_IndexAttrNumbers[i]))
                        return false;
 
+               /* collation and opfamily is not valid for including columns */
+               if (i >= info1->ii_NumIndexKeyAttrs)
+                       continue;
+
                if (collations1[i] != collations2[i])
                        return false;
                if (opfamilies1[i] != opfamilies2[i])

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 3d90204d420093253c4936e224dc8b6f19126bd4..61a4b24437b143b859964a7cd659615e4dd03838 100644 (file)
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -1359,7 +1359,8 @@ CheckPredicate(Expr *predicate)
 
 /*
  * Compute per-index-column information, including indexed column numbers
- * or index expressions, opclasses, and indoptions.
+ * or index expressions, opclasses, and indoptions. Note, all output vectors
+ * should be allocated for all columns, including "including" ones.
  */
 static void
 ComputeIndexAttrs(IndexInfo *indexInfo,
@@ -1490,6 +1491,36 @@ ComputeIndexAttrs(IndexInfo *indexInfo,
 
                typeOidP[attn] = atttype;
 
+               /*
+                * Included columns have no collation, no opclass and no ordering options.
+                */
+               if (attn >= nkeycols)
+               {
+                       if (attribute->collation)
+                               ereport(ERROR,
+                                               (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                                                errmsg("including column does not support a collation")));
+                       if (attribute->opclass)
+                               ereport(ERROR,
+                                               (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                                                errmsg("including column does not support an operator class")));
+                       if (attribute->ordering != SORTBY_DEFAULT)
+                               ereport(ERROR,
+                                               (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                                                errmsg("including column does not support ASC/DESC options")));
+                       if (attribute->nulls_ordering != SORTBY_NULLS_DEFAULT)
+                               ereport(ERROR,
+                                               (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                                                errmsg("including column does not support NULLS FIRST/LAST options")));
+
+                       classOidP[attn] = InvalidOid;
+                       colOptionP[attn] = 0;
+                       collationOidP[attn] = InvalidOid;
+                       attn++;
+
+                       continue;
+               }
+
                /*
                 * Apply collation override if any
                 */
@@ -1521,17 +1552,6 @@ ComputeIndexAttrs(IndexInfo *indexInfo,
 
                collationOidP[attn] = attcollation;
 
-               /*
-                * Included columns have no opclass and no ordering options.
-                */
-               if (attn >= nkeycols)
-               {
-                       classOidP[attn] = InvalidOid;
-                       colOptionP[attn] = 0;
-                       attn++;
-                       continue;
-               }
-
                /*
                 * Identify the opclass to use.
                 */

diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index bf42b54970c1e72ebbdff942b193b3fbd030ccb5..07d55a59ad6ffb2e5a41cfa11f0cc36deebfd0ed 100644 (file)
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -2329,8 +2329,8 @@ match_clause_to_indexcol(IndexOptInfo *index,
 {
        Expr       *clause = rinfo->clause;
        Index           index_relid = index->rel->relid;
-       Oid                     opfamily = index->opfamily[indexcol];
-       Oid                     idxcollation = index->indexcollations[indexcol];
+       Oid                     opfamily;
+       Oid                     idxcollation;
        Node       *leftop,
                           *rightop;
        Relids          left_relids;
@@ -2339,6 +2339,11 @@ match_clause_to_indexcol(IndexOptInfo *index,
        Oid                     expr_coll;
        bool            plain_op;
 
+       Assert(indexcol < index->nkeycolumns);
+
+       opfamily = index->opfamily[indexcol];
+       idxcollation = index->indexcollations[indexcol];
+
        /* First check for boolean-index cases. */
        if (IsBooleanOpfamily(opfamily))
        {
@@ -2678,8 +2683,8 @@ match_clause_to_ordering_op(IndexOptInfo *index,
                                                        Expr *clause,
                                                        Oid pk_opfamily)
 {
-       Oid                     opfamily = index->opfamily[indexcol];
-       Oid                     idxcollation = index->indexcollations[indexcol];
+       Oid                     opfamily;
+       Oid                     idxcollation;
        Node       *leftop,
                           *rightop;
        Oid                     expr_op;
@@ -2687,6 +2692,10 @@ match_clause_to_ordering_op(IndexOptInfo *index,
        Oid                     sortfamily;
        bool            commuted;
 
+       Assert(indexcol < index->nkeycolumns);
+
+       opfamily = index->opfamily[indexcol];
+       idxcollation = index->indexcollations[indexcol];
        /*
         * Clause must be a binary opclause.
         */
@@ -2921,8 +2930,13 @@ ec_member_matches_indexcol(PlannerInfo *root, RelOptInfo *rel,
 {
        IndexOptInfo *index = ((ec_member_matches_arg *) arg)->index;
        int                     indexcol = ((ec_member_matches_arg *) arg)->indexcol;
-       Oid                     curFamily = index->opfamily[indexcol];
-       Oid                     curCollation = index->indexcollations[indexcol];
+       Oid                     curFamily;
+       Oid                     curCollation;
+
+       Assert(indexcol < index->nkeycolumns);
+
+       curFamily = index->opfamily[indexcol];
+       curCollation = index->indexcollations[indexcol];
 
        /*
         * If it's a btree index, we can reject it if its opfamily isn't
@@ -3548,8 +3562,13 @@ expand_indexqual_conditions(IndexOptInfo *index,
                RestrictInfo *rinfo = (RestrictInfo *) lfirst(lcc);
                int                     indexcol = lfirst_int(lci);
                Expr       *clause = rinfo->clause;
-               Oid                     curFamily = index->opfamily[indexcol];
-               Oid                     curCollation = index->indexcollations[indexcol];
+               Oid                     curFamily;
+               Oid                     curCollation;
+
+               Assert(indexcol < index->nkeycolumns);
+
+               curFamily = index->opfamily[indexcol];
+               curCollation = index->indexcollations[indexcol];
 
                /* First check for boolean cases */
                if (IsBooleanOpfamily(curFamily))
@@ -3918,14 +3937,15 @@ adjust_rowcompare_for_index(RowCompareExpr *clause,
                /*
                 * The Var side can match any column of the index.
                 */
-               for (i = 0; i < index->ncolumns; i++)
+               for (i = 0; i < index->nkeycolumns; i++)
                {
                        if (match_index_to_operand(varop, i, index) &&
                                get_op_opfamily_strategy(expr_op,
                                                                                 index->opfamily[i]) == op_strategy &&
                                IndexCollMatchesExprColl(index->indexcollations[i],
                                                                                 lfirst_oid(collids_cell)))
-                               break;
+
+                       break;
                }
                if (i >= index->ncolumns)
                        break;                          /* no match found */

diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index 04ee5f201539e0c889f72666d691ebdc22eadb3b..cb9cfc372997740f01d7e10dc105c10de31da560 100644 (file)
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -242,7 +242,7 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent,
                        info->nkeycolumns = nkeycolumns = index->indnkeyatts;
 
                        info->indexkeys = (int *) palloc(sizeof(int) * ncolumns);
-                       info->indexcollations = (Oid *) palloc(sizeof(Oid) * ncolumns);
+                       info->indexcollations = (Oid *) palloc(sizeof(Oid) * nkeycolumns);
                        info->opfamily = (Oid *) palloc(sizeof(Oid) * nkeycolumns);
                        info->opcintype = (Oid *) palloc(sizeof(Oid) * nkeycolumns);
                        info->canreturn = (bool *) palloc(sizeof(bool) * ncolumns);
@@ -250,7 +250,6 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent,
                        for (i = 0; i < ncolumns; i++)
                        {
                                info->indexkeys[i] = index->indkey.values[i];
-                               info->indexcollations[i] = indexRelation->rd_indcollation[i];
                                info->canreturn[i] = index_can_return(indexRelation, i + 1);
                        }
 
@@ -258,6 +257,7 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent,
                        {
                                info->opfamily[i] = indexRelation->rd_opfamily[i];
                                info->opcintype[i] = indexRelation->rd_opcintype[i];
+                               info->indexcollations[i] = indexRelation->rd_indcollation[i];
                        }
 
                        info->relam = indexRelation->rd_rel->relam;

diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index f9f9904bad80763a9ed401cb80f0496bd004d27c..91781399127a96343ccfb21679ac02e0a1cf1afd 100644 (file)
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -1588,9 +1588,6 @@ generateClonedIndexStmt(RangeVar *heapRel, Oid heapRelid, Relation source_idx,
                /* Copy the original index column name */
                iparam->indexcolname = pstrdup(NameStr(attr->attname));
 
-               /* Add the collation name, if non-default */
-               iparam->collation = get_collation(indcollation->values[keyno], keycoltype);
-
                index->indexIncludingParams = lappend(index->indexIncludingParams, iparam);
        }
        /* Copy reloptions if any */

diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index b75a224ee8cca35572f5c63f1a2ea96bc63c6ac5..99643e83b25b8dd5622f7f2a3896df8df5c3c256 100644 (file)
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -1356,15 +1356,15 @@ pg_get_indexdef_worker(Oid indexrelid, int colno,
                {
                        Oid                     indcoll;
 
+                       if (keyno >= idxrec->indnkeyatts)
+                               continue;
+
                        /* Add collation, if not default for column */
                        indcoll = indcollation->values[keyno];
                        if (OidIsValid(indcoll) && indcoll != keycolcollation)
                                appendStringInfo(&buf, " COLLATE %s",
                                                                 generate_collation_name((indcoll)));
 
-                       if (keyno >= idxrec->indnkeyatts)
-                               continue;
-
                        /* Add the operator class name, if not default */
                        get_opclass_name(indclass->values[keyno], keycoltype, &buf);
 

diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index fe606d72798fa2e16151031e4c52b7abe2c72ea3..4b08cdb721aca0bc64647ffbac352cc47ac4c73a 100644 (file)
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -7437,6 +7437,8 @@ gincost_pattern(IndexOptInfo *index, int indexcol,
        int32           searchMode = GIN_SEARCH_MODE_DEFAULT;
        int32           i;
 
+       Assert(indexcol < index->nkeycolumns);
+
        /*
         * Get the operator's strategy number and declared input data types within
         * the index opfamily.  (We don't need the latter, but we use

diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index fd2a3cc8c3d7d0cb13761a393444046b1039a694..4939d1e34847700811e4996d3c8f94fef6a714ad 100644 (file)
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -1637,10 +1637,10 @@ RelationInitIndexAccessInfo(Relation relation)
        }
 
        relation->rd_indcollation = (Oid *)
-               MemoryContextAllocZero(indexcxt, indnatts * sizeof(Oid));
+               MemoryContextAllocZero(indexcxt, indnkeyatts * sizeof(Oid));
 
        relation->rd_indoption = (int16 *)
-               MemoryContextAllocZero(indexcxt, indnatts * sizeof(int16));
+               MemoryContextAllocZero(indexcxt, indnkeyatts * sizeof(int16));
 
        /*
         * indcollation cannot be referenced directly through the C struct,
@@ -1653,7 +1653,7 @@ RelationInitIndexAccessInfo(Relation relation)
                                                           &isnull);
        Assert(!isnull);
        indcoll = (oidvector *) DatumGetPointer(indcollDatum);
-       memcpy(relation->rd_indcollation, indcoll->values, indnatts * sizeof(Oid));
+       memcpy(relation->rd_indcollation, indcoll->values, indnkeyatts * sizeof(Oid));
 
        /*
         * indclass cannot be referenced directly through the C struct, because it
@@ -1685,7 +1685,7 @@ RelationInitIndexAccessInfo(Relation relation)
                                                                 &isnull);
        Assert(!isnull);
        indoption = (int2vector *) DatumGetPointer(indoptionDatum);
-       memcpy(relation->rd_indoption, indoption->values, indnatts * sizeof(int16));
+       memcpy(relation->rd_indoption, indoption->values, indnkeyatts * sizeof(int16));
 
        /*
         * expressions, predicate, exclusion caches will be filled later