From d87510a524f36a630cfb34cc392e95e959a1b0dc Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Fri, 30 Mar 2018 16:33:42 -0700 Subject: [PATCH] Combine options for RangeVarGetRelidExtended() into a flags argument. A followup patch will add a SKIP_LOCKED option. To avoid introducing evermore arguments, breaking existing callers each time, introduce a flags argument. This'll no doubt break a few external users... Also change the MISSING_OK behaviour so a DEBUG1 debug message is emitted when a relation is not found. Author: Nathan Bossart Reviewed-By: Michael Paquier and Andres Freund Discussion: https://postgr.es/m/20180306005349.b65whmvj7z6hbe2y@alap3.anarazel.de --- src/backend/catalog/namespace.c | 20 ++++++++++++-------- src/backend/commands/cluster.c | 2 +- src/backend/commands/indexcmds.c | 4 ++-- src/backend/commands/lockcmds.c | 4 ++-- src/backend/commands/matview.c | 2 +- src/backend/commands/policy.c | 6 +++--- src/backend/commands/sequence.c | 3 +-- src/backend/commands/tablecmds.c | 16 ++++++++-------- src/backend/commands/trigger.c | 2 +- src/backend/rewrite/rewriteDefine.c | 2 +- src/backend/tcop/utility.c | 2 +- src/include/catalog/namespace.h | 14 ++++++++++++-- 12 files changed, 45 insertions(+), 32 deletions(-) diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c index 52dd400b96..687d31e083 100644 --- a/src/backend/catalog/namespace.c +++ b/src/backend/catalog/namespace.c @@ -206,23 +206,25 @@ static bool MatchNamedCall(HeapTuple proctup, int nargs, List *argnames, * Given a RangeVar describing an existing relation, * select the proper namespace and look up the relation OID. * - * If the schema or relation is not found, return InvalidOid if missing_ok - * = true, otherwise raise an error. + * If the schema or relation is not found, return InvalidOid if flags contains + * RVR_MISSING_OK, otherwise raise an error. * - * If nowait = true, throw an error if we'd have to wait for a lock. + * If flags contains RVR_NOWAIT, throw an error if we'd have to wait for a + * lock. * * Callback allows caller to check permissions or acquire additional locks * prior to grabbing the relation lock. */ Oid RangeVarGetRelidExtended(const RangeVar *relation, LOCKMODE lockmode, - bool missing_ok, bool nowait, + uint32 flags, RangeVarGetRelidCallback callback, void *callback_arg) { uint64 inval_count; Oid relId; Oid oldRelId = InvalidOid; bool retry = false; + bool missing_ok = (flags & RVR_MISSING_OK) != 0; /* * We check the catalog name and then ignore it. @@ -361,7 +363,7 @@ RangeVarGetRelidExtended(const RangeVar *relation, LOCKMODE lockmode, */ if (!OidIsValid(relId)) AcceptInvalidationMessages(); - else if (!nowait) + else if (!(flags & RVR_NOWAIT)) LockRelationOid(relId, lockmode); else if (!ConditionalLockRelationOid(relId, lockmode)) { @@ -392,15 +394,17 @@ RangeVarGetRelidExtended(const RangeVar *relation, LOCKMODE lockmode, oldRelId = relId; } - if (!OidIsValid(relId) && !missing_ok) + if (!OidIsValid(relId)) { + int elevel = missing_ok ? DEBUG1 : ERROR; + if (relation->schemaname) - ereport(ERROR, + ereport(elevel, (errcode(ERRCODE_UNDEFINED_TABLE), errmsg("relation \"%s.%s\" does not exist", relation->schemaname, relation->relname))); else - ereport(ERROR, + ereport(elevel, (errcode(ERRCODE_UNDEFINED_TABLE), errmsg("relation \"%s\" does not exist", relation->relname))); diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 0f844c00c8..d088dc11a6 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -115,7 +115,7 @@ cluster(ClusterStmt *stmt, bool isTopLevel) /* Find, lock, and check permissions on the table */ tableOid = RangeVarGetRelidExtended(stmt->relation, AccessExclusiveLock, - false, false, + 0, RangeVarCallbackOwnsTable, NULL); rel = heap_open(tableOid, NoLock); diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 0185970794..e224b91f53 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -2091,7 +2091,7 @@ ReindexIndex(RangeVar *indexRelation, int options) * used here must match the index lock obtained in reindex_index(). */ indOid = RangeVarGetRelidExtended(indexRelation, AccessExclusiveLock, - false, false, + 0, RangeVarCallbackForReindexIndex, (void *) &heapOid); @@ -2183,7 +2183,7 @@ ReindexTable(RangeVar *relation, int options) Oid heapOid; /* The lock level used here should match reindex_relation(). */ - heapOid = RangeVarGetRelidExtended(relation, ShareLock, false, false, + heapOid = RangeVarGetRelidExtended(relation, ShareLock, 0, RangeVarCallbackOwnsTable, NULL); if (!reindex_relation(heapOid, diff --git a/src/backend/commands/lockcmds.c b/src/backend/commands/lockcmds.c index 7f25d7498a..1dbb35f631 100644 --- a/src/backend/commands/lockcmds.c +++ b/src/backend/commands/lockcmds.c @@ -61,8 +61,8 @@ LockTableCommand(LockStmt *lockstmt) bool recurse = rv->inh; Oid reloid; - reloid = RangeVarGetRelidExtended(rv, lockstmt->mode, false, - lockstmt->nowait, + reloid = RangeVarGetRelidExtended(rv, lockstmt->mode, + lockstmt->nowait ? RVR_NOWAIT : 0, RangeVarCallbackForLockTable, (void *) &lockstmt->mode); diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c index 23892b1b81..410d4e5a38 100644 --- a/src/backend/commands/matview.c +++ b/src/backend/commands/matview.c @@ -161,7 +161,7 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString, * Get a lock until end of transaction. */ matviewOid = RangeVarGetRelidExtended(stmt->relation, - lockmode, false, false, + lockmode, 0, RangeVarCallbackOwnsTable, NULL); matviewRel = heap_open(matviewOid, NoLock); diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c index cfaf32ccbd..00841b3b8a 100644 --- a/src/backend/commands/policy.c +++ b/src/backend/commands/policy.c @@ -743,7 +743,7 @@ CreatePolicy(CreatePolicyStmt *stmt) /* Get id of table. Also handles permissions checks. */ table_id = RangeVarGetRelidExtended(stmt->table, AccessExclusiveLock, - false, false, + 0, RangeVarCallbackForPolicy, (void *) stmt); @@ -915,7 +915,7 @@ AlterPolicy(AlterPolicyStmt *stmt) /* Get id of table. Also handles permissions checks. */ table_id = RangeVarGetRelidExtended(stmt->table, AccessExclusiveLock, - false, false, + 0, RangeVarCallbackForPolicy, (void *) stmt); @@ -1215,7 +1215,7 @@ rename_policy(RenameStmt *stmt) /* Get id of table. Also handles permissions checks. */ table_id = RangeVarGetRelidExtended(stmt->relation, AccessExclusiveLock, - false, false, + 0, RangeVarCallbackForPolicy, (void *) stmt); diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c index dcc0aa536a..89122d4ad7 100644 --- a/src/backend/commands/sequence.c +++ b/src/backend/commands/sequence.c @@ -432,8 +432,7 @@ AlterSequence(ParseState *pstate, AlterSeqStmt *stmt) /* Open and lock sequence, and check for ownership along the way. */ relid = RangeVarGetRelidExtended(stmt->sequence, ShareRowExclusiveLock, - stmt->missing_ok, - false, + stmt->missing_ok ? RVR_MISSING_OK : 0, RangeVarCallbackOwnsRelation, NULL); if (relid == InvalidOid) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 83a881eff3..c8da82217d 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -1159,8 +1159,7 @@ RemoveRelations(DropStmt *drop) state.heapOid = InvalidOid; state.partParentOid = InvalidOid; state.concurrent = drop->concurrent; - relOid = RangeVarGetRelidExtended(rel, lockmode, true, - false, + relOid = RangeVarGetRelidExtended(rel, lockmode, RVR_MISSING_OK, RangeVarCallbackForDropRelation, (void *) &state); @@ -2793,7 +2792,7 @@ renameatt(RenameStmt *stmt) /* lock level taken here should match renameatt_internal */ relid = RangeVarGetRelidExtended(stmt->relation, AccessExclusiveLock, - stmt->missing_ok, false, + stmt->missing_ok ? RVR_MISSING_OK : 0, RangeVarCallbackForRenameAttribute, NULL); @@ -2945,7 +2944,7 @@ RenameConstraint(RenameStmt *stmt) { /* lock level taken here should match rename_constraint_internal */ relid = RangeVarGetRelidExtended(stmt->relation, AccessExclusiveLock, - stmt->missing_ok, false, + stmt->missing_ok ? RVR_MISSING_OK : 0, RangeVarCallbackForRenameAttribute, NULL); if (!OidIsValid(relid)) @@ -2987,7 +2986,7 @@ RenameRelation(RenameStmt *stmt) * escalation. */ relid = RangeVarGetRelidExtended(stmt->relation, AccessExclusiveLock, - stmt->missing_ok, false, + stmt->missing_ok ? RVR_MISSING_OK : 0, RangeVarCallbackForAlterRelation, (void *) stmt); @@ -3146,7 +3145,8 @@ CheckTableNotInUse(Relation rel, const char *stmt) Oid AlterTableLookupRelation(AlterTableStmt *stmt, LOCKMODE lockmode) { - return RangeVarGetRelidExtended(stmt->relation, lockmode, stmt->missing_ok, false, + return RangeVarGetRelidExtended(stmt->relation, lockmode, + stmt->missing_ok ? RVR_MISSING_OK : 0, RangeVarCallbackForAlterRelation, (void *) stmt); } @@ -12683,7 +12683,7 @@ AlterTableNamespace(AlterObjectSchemaStmt *stmt, Oid *oldschema) ObjectAddress myself; relid = RangeVarGetRelidExtended(stmt->relation, AccessExclusiveLock, - stmt->missing_ok, false, + stmt->missing_ok ? RVR_MISSING_OK : 0, RangeVarCallbackForAlterRelation, (void *) stmt); @@ -14613,7 +14613,7 @@ ATExecAttachPartitionIdx(List **wqueue, Relation parentIdx, RangeVar *name) state.parentTblOid = parentIdx->rd_index->indrelid; state.lockedParentTbl = false; partIdxId = - RangeVarGetRelidExtended(name, AccessExclusiveLock, false, false, + RangeVarGetRelidExtended(name, AccessExclusiveLock, 0, RangeVarCallbackForAttachIndex, (void *) &state); /* Not there? */ diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 9d8df5986e..a6593f939c 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -1667,7 +1667,7 @@ renametrig(RenameStmt *stmt) * release until end of transaction). */ relid = RangeVarGetRelidExtended(stmt->relation, AccessExclusiveLock, - false, false, + 0, RangeVarCallbackForRenameTrigger, NULL); diff --git a/src/backend/rewrite/rewriteDefine.c b/src/backend/rewrite/rewriteDefine.c index 679be605f1..d81a2ea342 100644 --- a/src/backend/rewrite/rewriteDefine.c +++ b/src/backend/rewrite/rewriteDefine.c @@ -951,7 +951,7 @@ RenameRewriteRule(RangeVar *relation, const char *oldName, * release until end of transaction). */ relid = RangeVarGetRelidExtended(relation, AccessExclusiveLock, - false, false, + 0, RangeVarCallbackForRenameRule, NULL); diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index d355bef606..b2dc9d18ea 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -1305,7 +1305,7 @@ ProcessUtilitySlow(ParseState *pstate, : ShareLock; relid = RangeVarGetRelidExtended(stmt->relation, lockmode, - false, false, + 0, RangeVarCallbackOwnsRelation, NULL); diff --git a/src/include/catalog/namespace.h b/src/include/catalog/namespace.h index 5f8cf4992e..0bccf910a9 100644 --- a/src/include/catalog/namespace.h +++ b/src/include/catalog/namespace.h @@ -47,14 +47,24 @@ typedef struct OverrideSearchPath bool addTemp; /* implicitly prepend temp schema? */ } OverrideSearchPath; +/* + * Option flag bits for RangeVarGetRelidExtended(). + */ +typedef enum RVROption +{ + RVR_MISSING_OK = 1 << 0, /* don't error if relation doesn't exist */ + RVR_NOWAIT = 1 << 1 /* error if relation cannot be locked */ +} RVROption; + typedef void (*RangeVarGetRelidCallback) (const RangeVar *relation, Oid relId, Oid oldRelId, void *callback_arg); #define RangeVarGetRelid(relation, lockmode, missing_ok) \ - RangeVarGetRelidExtended(relation, lockmode, missing_ok, false, NULL, NULL) + RangeVarGetRelidExtended(relation, lockmode, \ + (missing_ok) ? RVR_MISSING_OK : 0, NULL, NULL) extern Oid RangeVarGetRelidExtended(const RangeVar *relation, - LOCKMODE lockmode, bool missing_ok, bool nowait, + LOCKMODE lockmode, uint32 flags, RangeVarGetRelidCallback callback, void *callback_arg); extern Oid RangeVarGetCreationNamespace(const RangeVar *newRelation);