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);