From 37e683e352df5f3e6c110d94881abe888e36953e Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Mon, 16 Oct 2017 12:01:12 +0200 Subject: [PATCH v2 1/7] Tweak index_create/index_constraint_create APIs I was annoyed by index_create and index_constraint_create respective APIs. It's too easy to get confused when adding a new behavioral flag given the large number of boolean flags they already have. Turning them into a flags bitmask makes that code easier to read, as in the attached patch. I split the flags in two -- one set for index_create directly and another related to constraints. index_create() itself receives both, and then passes down the constraint one to index_constraint_create. It is shorter in LOC terms to create a single set mixing all the values, but it seemed to make more sense this way. Also, I chose not to turn the booleans 'allow_sytem_table_mods' and 'is_internal' into flag bits because of the different way they interact with this code. --- src/backend/catalog/index.c | 101 +++++++++++++++++++-------------------- src/backend/catalog/toasting.c | 3 +- src/backend/commands/indexcmds.c | 33 +++++++++---- src/backend/commands/tablecmds.c | 13 +++-- src/include/catalog/index.h | 29 ++++++----- 5 files changed, 100 insertions(+), 79 deletions(-) diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index c7b2f031f0..17faeffada 100644 --- 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, + uint16 flags, + uint16 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; + bool concurrent = flags & INDEX_CREATE_CONCURRENT; + + /* constraint flags can only be set when 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, + uint16 constr_flags, bool allow_system_table_mods, bool is_internal) { @@ -1164,6 +1159,9 @@ index_constraint_create(Relation heapRelation, ObjectAddress myself, referenced; Oid conOid; + bool deferrable = constr_flags & INDEX_CONSTR_CREATE_DEFERRABLE; + bool initdeferred = constr_flags & INDEX_CONSTR_CREATE_INIT_DEFERRED; + bool mark_as_primary = constr_flags & INDEX_CONSTR_CREATE_MARK_AS_PRIMARY; /* constraint creation support doesn't work while bootstrapping */ Assert(!IsBootstrapProcessingMode()); @@ -1190,7 +1188,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 +1293,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 6f517bbcda..539ca79ad3 100644 --- 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 3f615b6260..92c09b06dd 100644 --- 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; + uint16 flags; + uint16 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 2d4dcd7556..360027a06c 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -6823,6 +6823,7 @@ ATExecAddIndexConstraint(AlteredTableInfo *tab, Relation rel, char *constraintName; char constraintType; ObjectAddress address; + uint16 flags; Assert(IsA(stmt, IndexStmt)); Assert(OidIsValid(index_oid)); @@ -6867,16 +6868,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 1d4ec09f8f..ab1f9ef1bd 100644 --- 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, + uint16 flags, + uint16 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, + uint16 constr_flags, bool allow_system_table_mods, bool is_internal); -- 2.11.0