Simplify index_[constraint_]create API

Instead of passing large swaths of boolean arguments, define some flags
that can be used in a bitmask.  This makes it easier not only to figure
out what each call site is doing, but also to add some new flags.

The flags are split in two -- one set for index_create directly and
another for constraints.  index_create() itself receives both, and then
passes down the latter to index_constraint_create(), which can also be
called standalone.

Discussion: https://postgr.es/m/20171023151251.j75uoe27gajdjmlm@alvherre.pgsql
Reviewed-by: Simon Riggs

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index c7b2f031f09dd35478b01d7696829ed96eee7d3f..0125c18bc16c911ace4230bf2d3be06657db486e 100644 (file)
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -680,19 +680,25 @@ UpdateIndexRelation(Oid indexoid,
  * classObjectId: array of index opclass OIDs, one per index column
  * coloptions: array of per-index-column indoption settings
  * reloptions: AM-specific options
- * isprimary: index is a PRIMARY KEY
- * isconstraint: index is owned by PRIMARY KEY, UNIQUE, or EXCLUSION constraint
- * deferrable: constraint is DEFERRABLE
- * initdeferred: constraint is INITIALLY DEFERRED
+ * flags: bitmask that can include any combination of these bits:
+ *             INDEX_CREATE_IS_PRIMARY
+ *                     the index is a primary key
+ *             INDEX_CREATE_ADD_CONSTRAINT:
+ *                     invoke index_constraint_create also
+ *             INDEX_CREATE_SKIP_BUILD:
+ *                     skip the index_build() step for the moment; caller must do it
+ *                     later (typically via reindex_index())
+ *             INDEX_CREATE_CONCURRENT:
+ *                     do not lock the table against writers.  The index will be
+ *                     marked "invalid" and the caller must take additional steps
+ *                     to fix it up.
+ *             INDEX_CREATE_IF_NOT_EXISTS:
+ *                     do not throw an error if a relation with the same name
+ *                     already exists.
+ * constr_flags: flags passed to index_constraint_create
+ *             (only if INDEX_CREATE_ADD_CONSTRAINT is set)
  * allow_system_table_mods: allow table to be a system catalog
- * skip_build: true to skip the index_build() step for the moment; caller
- *             must do it later (typically via reindex_index())
- * concurrent: if true, do not lock the table against writers.  The index
- *             will be marked "invalid" and the caller must take additional steps
- *             to fix it up.
  * is_internal: if true, post creation hook for new index
- * if_not_exists: if true, do not throw an error if a relation with
- *             the same name already exists.
  *
  * Returns the OID of the created index.
  */
@@ -709,15 +715,10 @@ index_create(Relation heapRelation,
                         Oid *classObjectId,
                         int16 *coloptions,
                         Datum reloptions,
-                        bool isprimary,
-                        bool isconstraint,
-                        bool deferrable,
-                        bool initdeferred,
+                        bits16 flags,
+                        bits16 constr_flags,
                         bool allow_system_table_mods,
-                        bool skip_build,
-                        bool concurrent,
-                        bool is_internal,
-                        bool if_not_exists)
+                        bool is_internal)
 {
        Oid                     heapRelationId = RelationGetRelid(heapRelation);
        Relation        pg_class;
@@ -729,6 +730,12 @@ index_create(Relation heapRelation,
        Oid                     namespaceId;
        int                     i;
        char            relpersistence;
+       bool            isprimary = (flags & INDEX_CREATE_IS_PRIMARY) != 0;
+       bool            concurrent = (flags & INDEX_CREATE_CONCURRENT) != 0;
+
+       /* constraint flags can only be set when a constraint is requested */
+       Assert((constr_flags == 0) ||
+                  ((flags & INDEX_CREATE_ADD_CONSTRAINT) != 0));
 
        is_exclusion = (indexInfo->ii_ExclusionOps != NULL);
 
@@ -794,7 +801,7 @@ index_create(Relation heapRelation,
 
        if (get_relname_relid(indexRelationName, namespaceId))
        {
-               if (if_not_exists)
+               if ((flags & INDEX_CREATE_IF_NOT_EXISTS) != 0)
                {
                        ereport(NOTICE,
                                        (errcode(ERRCODE_DUPLICATE_TABLE),
@@ -917,7 +924,7 @@ index_create(Relation heapRelation,
        UpdateIndexRelation(indexRelationId, heapRelationId, indexInfo,
                                                collationObjectId, classObjectId, coloptions,
                                                isprimary, is_exclusion,
-                                               !deferrable,
+                                               (constr_flags & INDEX_CONSTR_CREATE_DEFERRABLE) == 0,
                                                !concurrent);
 
        /*
@@ -943,7 +950,7 @@ index_create(Relation heapRelation,
                myself.objectId = indexRelationId;
                myself.objectSubId = 0;
 
-               if (isconstraint)
+               if ((flags & INDEX_CREATE_ADD_CONSTRAINT) != 0)
                {
                        char            constraintType;
 
@@ -964,11 +971,7 @@ index_create(Relation heapRelation,
                                                                        indexInfo,
                                                                        indexRelationName,
                                                                        constraintType,
-                                                                       deferrable,
-                                                                       initdeferred,
-                                                                       false,  /* already marked primary */
-                                                                       false,  /* pg_index entry is OK */
-                                                                       false,  /* no old dependencies */
+                                                                       constr_flags,
                                                                        allow_system_table_mods,
                                                                        is_internal);
                }
@@ -1005,10 +1008,6 @@ index_create(Relation heapRelation,
 
                                recordDependencyOn(&myself, &referenced, DEPENDENCY_AUTO);
                        }
-
-                       /* Non-constraint indexes can't be deferrable */
-                       Assert(!deferrable);
-                       Assert(!initdeferred);
                }
 
                /* Store dependency on collations */
@@ -1059,9 +1058,7 @@ index_create(Relation heapRelation,
        else
        {
                /* Bootstrap mode - assert we weren't asked for constraint support */
-               Assert(!isconstraint);
-               Assert(!deferrable);
-               Assert(!initdeferred);
+               Assert((flags & INDEX_CREATE_ADD_CONSTRAINT) == 0);
        }
 
        /* Post creation hook for new index */
@@ -1089,15 +1086,16 @@ index_create(Relation heapRelation,
         * If this is bootstrap (initdb) time, then we don't actually fill in the
         * index yet.  We'll be creating more indexes and classes later, so we
         * delay filling them in until just before we're done with bootstrapping.
-        * Similarly, if the caller specified skip_build then filling the index is
-        * delayed till later (ALTER TABLE can save work in some cases with this).
-        * Otherwise, we call the AM routine that constructs the index.
+        * Similarly, if the caller specified to skip the build then filling the
+        * index is delayed till later (ALTER TABLE can save work in some cases
+        * with this).  Otherwise, we call the AM routine that constructs the
+        * index.
         */
        if (IsBootstrapProcessingMode())
        {
                index_register(heapRelationId, indexRelationId, indexInfo);
        }
-       else if (skip_build)
+       else if ((flags & INDEX_CREATE_SKIP_BUILD) != 0)
        {
                /*
                 * Caller is responsible for filling the index later on.  However,
@@ -1137,12 +1135,13 @@ index_create(Relation heapRelation,
  * constraintName: what it say (generally, should match name of index)
  * constraintType: one of CONSTRAINT_PRIMARY, CONSTRAINT_UNIQUE, or
  *             CONSTRAINT_EXCLUSION
- * deferrable: constraint is DEFERRABLE
- * initdeferred: constraint is INITIALLY DEFERRED
- * mark_as_primary: if true, set flags to mark index as primary key
- * update_pgindex: if true, update pg_index row (else caller's done that)
- * remove_old_dependencies: if true, remove existing dependencies of index
- *             on table's columns
+ * flags: bitmask that can include any combination of these bits:
+ *             INDEX_CONSTR_CREATE_MARK_AS_PRIMARY: index is a PRIMARY KEY
+ *             INDEX_CONSTR_CREATE_DEFERRABLE: constraint is DEFERRABLE
+ *             INDEX_CONSTR_CREATE_INIT_DEFERRED: constraint is INITIALLY DEFERRED
+ *             INDEX_CONSTR_CREATE_UPDATE_INDEX: update the pg_index row
+ *             INDEX_CONSTR_CREATE_REMOVE_OLD_DEPS: remove existing dependencies
+ *                     of index on table's columns
  * allow_system_table_mods: allow table to be a system catalog
  * is_internal: index is constructed due to internal process
  */
@@ -1152,11 +1151,7 @@ index_constraint_create(Relation heapRelation,
                                                IndexInfo *indexInfo,
                                                const char *constraintName,
                                                char constraintType,
-                                               bool deferrable,
-                                               bool initdeferred,
-                                               bool mark_as_primary,
-                                               bool update_pgindex,
-                                               bool remove_old_dependencies,
+                                               bits16 constr_flags,
                                                bool allow_system_table_mods,
                                                bool is_internal)
 {
@@ -1164,6 +1159,13 @@ index_constraint_create(Relation heapRelation,
        ObjectAddress myself,
                                referenced;
        Oid                     conOid;
+       bool            deferrable;
+       bool            initdeferred;
+       bool            mark_as_primary;
+
+       deferrable = (constr_flags & INDEX_CONSTR_CREATE_DEFERRABLE) != 0;
+       initdeferred = (constr_flags & INDEX_CONSTR_CREATE_INIT_DEFERRED) != 0;
+       mark_as_primary = (constr_flags & INDEX_CONSTR_CREATE_MARK_AS_PRIMARY) != 0;
 
        /* constraint creation support doesn't work while bootstrapping */
        Assert(!IsBootstrapProcessingMode());
@@ -1190,7 +1192,7 @@ index_constraint_create(Relation heapRelation,
         * has any expressions or predicate, but we'd never be turning such an
         * index into a UNIQUE or PRIMARY KEY constraint.
         */
-       if (remove_old_dependencies)
+       if (constr_flags & INDEX_CONSTR_CREATE_REMOVE_OLD_DEPS)
                deleteDependencyRecordsForClass(RelationRelationId, indexRelationId,
                                                                                RelationRelationId, DEPENDENCY_AUTO);
 
@@ -1295,7 +1297,8 @@ index_constraint_create(Relation heapRelation,
         * is a risk that concurrent readers of the table will miss seeing this
         * index at all.
         */
-       if (update_pgindex && (mark_as_primary || deferrable))
+       if ((constr_flags & INDEX_CONSTR_CREATE_UPDATE_INDEX) &&
+               (mark_as_primary || deferrable))
        {
                Relation        pg_index;
                HeapTuple       indexTuple;

diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c
index 6f517bbcdaeaf26199a368454fb47a5bb4d9d50c..539ca79ad3b037a8ae7be4343139c941eb6ff1da 100644 (file)
--- a/src/backend/catalog/toasting.c
+++ b/src/backend/catalog/toasting.c
@@ -333,8 +333,7 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid,
                                 BTREE_AM_OID,
                                 rel->rd_rel->reltablespace,
                                 collationObjectId, classObjectId, coloptions, (Datum) 0,
-                                true, false, false, false,
-                                true, false, false, true, false);
+                                INDEX_CREATE_IS_PRIMARY, 0, true, true);
 
        heap_close(toast_rel, NoLock);
 

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 89114af119e377f5e7cf3ad52cb0b65598b25965..97091dd9fbd98b079aecc9a986d039420e9a0dd8 100644 (file)
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -333,6 +333,8 @@ DefineIndex(Oid relationId,
        Datum           reloptions;
        int16      *coloptions;
        IndexInfo  *indexInfo;
+       bits16          flags;
+       bits16          constr_flags;
        int                     numberOfAttributes;
        TransactionId limitXmin;
        VirtualTransactionId *old_snapshots;
@@ -661,20 +663,35 @@ DefineIndex(Oid relationId,
        Assert(!OidIsValid(stmt->oldNode) || (skip_build && !stmt->concurrent));
 
        /*
-        * Make the catalog entries for the index, including constraints. Then, if
-        * not skip_build || concurrent, actually build the index.
+        * Make the catalog entries for the index, including constraints. This
+        * step also actually builds the index, except if caller requested not to
+        * or in concurrent mode, in which case it'll be done later.
         */
+       flags = constr_flags = 0;
+       if (stmt->isconstraint)
+               flags |= INDEX_CREATE_ADD_CONSTRAINT;
+       if (skip_build || stmt->concurrent)
+               flags |= INDEX_CREATE_SKIP_BUILD;
+       if (stmt->if_not_exists)
+               flags |= INDEX_CREATE_IF_NOT_EXISTS;
+       if (stmt->concurrent)
+               flags |= INDEX_CREATE_CONCURRENT;
+       if (stmt->primary)
+               flags |= INDEX_CREATE_IS_PRIMARY;
+
+       if (stmt->deferrable)
+               constr_flags |= INDEX_CONSTR_CREATE_DEFERRABLE;
+       if (stmt->initdeferred)
+               constr_flags |= INDEX_CONSTR_CREATE_INIT_DEFERRED;
+
        indexRelationId =
                index_create(rel, indexRelationName, indexRelationId, stmt->oldNode,
                                         indexInfo, indexColNames,
                                         accessMethodId, tablespaceId,
                                         collationObjectId, classObjectId,
-                                        coloptions, reloptions, stmt->primary,
-                                        stmt->isconstraint, stmt->deferrable, stmt->initdeferred,
-                                        allowSystemTableMods,
-                                        skip_build || stmt->concurrent,
-                                        stmt->concurrent, !check_rights,
-                                        stmt->if_not_exists);
+                                        coloptions, reloptions,
+                                        flags, constr_flags,
+                                        allowSystemTableMods, !check_rights);
 
        ObjectAddressSet(address, RelationRelationId, indexRelationId);
 

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 9c66aa75ed32323eaa6f09eec6b436e0ea0fc47e..d19846d0057d96f7a8aa07d6e6b2535ff7fbac0a 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -6836,6 +6836,7 @@ ATExecAddIndexConstraint(AlteredTableInfo *tab, Relation rel,
        char       *constraintName;
        char            constraintType;
        ObjectAddress address;
+       bits16          flags;
 
        Assert(IsA(stmt, IndexStmt));
        Assert(OidIsValid(index_oid));
@@ -6880,16 +6881,18 @@ ATExecAddIndexConstraint(AlteredTableInfo *tab, Relation rel,
                constraintType = CONSTRAINT_UNIQUE;
 
        /* Create the catalog entries for the constraint */
+       flags = INDEX_CONSTR_CREATE_UPDATE_INDEX |
+               INDEX_CONSTR_CREATE_REMOVE_OLD_DEPS |
+               (stmt->initdeferred ? INDEX_CONSTR_CREATE_INIT_DEFERRED : 0) |
+               (stmt->deferrable ? INDEX_CONSTR_CREATE_DEFERRABLE : 0) |
+               (stmt->primary ? INDEX_CONSTR_CREATE_MARK_AS_PRIMARY : 0);
+
        address = index_constraint_create(rel,
                                                                          index_oid,
                                                                          indexInfo,
                                                                          constraintName,
                                                                          constraintType,
-                                                                         stmt->deferrable,
-                                                                         stmt->initdeferred,
-                                                                         stmt->primary,
-                                                                         true, /* update pg_index */
-                                                                         true, /* remove old dependencies */
+                                                                         flags,
                                                                          allowSystemTableMods,
                                                                          false);       /* is_internal */
 

diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index 1d4ec09f8fbd3a6d52f0623646fb05433e0f56d5..ceaa91f1b2e30fdfbe81a6890cd8a12c55d05fea 100644 (file)
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -42,6 +42,12 @@ extern void index_check_primary_key(Relation heapRel,
                                                IndexInfo *indexInfo,
                                                bool is_alter_table);
 
+#define        INDEX_CREATE_IS_PRIMARY                         (1 << 0)
+#define        INDEX_CREATE_ADD_CONSTRAINT                     (1 << 1)
+#define        INDEX_CREATE_SKIP_BUILD                         (1 << 2)
+#define        INDEX_CREATE_CONCURRENT                         (1 << 3)
+#define        INDEX_CREATE_IF_NOT_EXISTS                      (1 << 4)
+
 extern Oid index_create(Relation heapRelation,
                         const char *indexRelationName,
                         Oid indexRelationId,
@@ -54,26 +60,23 @@ extern Oid index_create(Relation heapRelation,
                         Oid *classObjectId,
                         int16 *coloptions,
                         Datum reloptions,
-                        bool isprimary,
-                        bool isconstraint,
-                        bool deferrable,
-                        bool initdeferred,
+                        bits16 flags,
+                        bits16 constr_flags,
                         bool allow_system_table_mods,
-                        bool skip_build,
-                        bool concurrent,
-                        bool is_internal,
-                        bool if_not_exists);
+                        bool is_internal);
+
+#define        INDEX_CONSTR_CREATE_MARK_AS_PRIMARY     (1 << 0)
+#define        INDEX_CONSTR_CREATE_DEFERRABLE          (1 << 1)
+#define        INDEX_CONSTR_CREATE_INIT_DEFERRED       (1 << 2)
+#define        INDEX_CONSTR_CREATE_UPDATE_INDEX        (1 << 3)
+#define        INDEX_CONSTR_CREATE_REMOVE_OLD_DEPS     (1 << 4)
 
 extern ObjectAddress index_constraint_create(Relation heapRelation,
                                                Oid indexRelationId,
                                                IndexInfo *indexInfo,
                                                const char *constraintName,
                                                char constraintType,
-                                               bool deferrable,
-                                               bool initdeferred,
-                                               bool mark_as_primary,
-                                               bool update_pgindex,
-                                               bool remove_old_dependencies,
+                                               bits16 constr_flags,
                                                bool allow_system_table_mods,
                                                bool is_internal);