From: Tom Lane Date: Sun, 14 May 2017 17:32:59 +0000 (-0400) Subject: Fix maintenance hazards caused by ill-considered use of default: cases. X-Git-Tag: REL_10_BETA1~18 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=e84c0195980f24b1c7f857b88834c1dcaf20a102;p=postgresql Fix maintenance hazards caused by ill-considered use of default: cases. Remove default cases from assorted switches over ObjectClass and some related enum types, so that we'll get compiler warnings when someone adds a new enum value without accounting for it in all these places. In passing, re-order some switch cases as needed to match the declaration of enum ObjectClass. OK, that's just neatnik-ism, but I dislike code that looks like it was assembled with the help of a dartboard. Discussion: https://postgr.es/m/20170512221010.nglatgt5azzdxjlj@alvherre.pgsql --- diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c index cdf453562f..806db7f35e 100644 --- a/src/backend/catalog/dependency.c +++ b/src/backend/catalog/dependency.c @@ -1204,6 +1204,10 @@ doDeletion(const ObjectAddress *object, int flags) RemoveSchemaById(object->objectId); break; + case OCLASS_STATISTIC_EXT: + RemoveStatisticsById(object->objectId); + break; + case OCLASS_TSPARSER: RemoveTSParserById(object->objectId); break; @@ -1265,13 +1269,20 @@ doDeletion(const ObjectAddress *object, int flags) DropTransformById(object->objectId); break; - case OCLASS_STATISTIC_EXT: - RemoveStatisticsById(object->objectId); + /* + * These global object types are not supported here. + */ + case OCLASS_ROLE: + case OCLASS_DATABASE: + case OCLASS_TBLSPACE: + case OCLASS_SUBSCRIPTION: + elog(ERROR, "global objects cannot be deleted by doDeletion"); break; - default: - elog(ERROR, "unrecognized object class: %u", - object->classId); + /* + * There's intentionally no default: case here; we want the + * compiler to warn if a new OCLASS hasn't been handled above. + */ } } diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c index e3efb98b1d..3dfb8fa4f9 100644 --- a/src/backend/catalog/objectaddress.c +++ b/src/backend/catalog/objectaddress.c @@ -2869,6 +2869,21 @@ getObjectDescription(const ObjectAddress *object) getOpFamilyDescription(&buffer, object->objectId); break; + case OCLASS_AM: + { + HeapTuple tup; + + tup = SearchSysCache1(AMOID, + ObjectIdGetDatum(object->objectId)); + if (!HeapTupleIsValid(tup)) + elog(ERROR, "cache lookup failed for access method %u", + object->objectId); + appendStringInfo(&buffer, _("access method %s"), + NameStr(((Form_pg_am) GETSTRUCT(tup))->amname)); + ReleaseSysCache(tup); + break; + } + case OCLASS_AMOP: { Relation amopDesc; @@ -3004,47 +3019,6 @@ getObjectDescription(const ObjectAddress *object) break; } - case OCLASS_STATISTIC_EXT: - { - HeapTuple stxTup; - Form_pg_statistic_ext stxForm; - - stxTup = SearchSysCache1(STATEXTOID, - ObjectIdGetDatum(object->objectId)); - if (!HeapTupleIsValid(stxTup)) - elog(ERROR, "could not find tuple for statistics object %u", - object->objectId); - - stxForm = (Form_pg_statistic_ext) GETSTRUCT(stxTup); - - appendStringInfo(&buffer, _("statistics object %s"), - NameStr(stxForm->stxname)); - - ReleaseSysCache(stxTup); - break; - } - - case OCLASS_TRANSFORM: - { - HeapTuple trfTup; - Form_pg_transform trfForm; - - trfTup = SearchSysCache1(TRFOID, - ObjectIdGetDatum(object->objectId)); - if (!HeapTupleIsValid(trfTup)) - elog(ERROR, "could not find tuple for transform %u", - object->objectId); - - trfForm = (Form_pg_transform) GETSTRUCT(trfTup); - - appendStringInfo(&buffer, _("transform for %s language %s"), - format_type_be(trfForm->trftype), - get_language_name(trfForm->trflang, false)); - - ReleaseSysCache(trfTup); - break; - } - case OCLASS_TRIGGER: { Relation trigDesc; @@ -3092,6 +3066,26 @@ getObjectDescription(const ObjectAddress *object) break; } + case OCLASS_STATISTIC_EXT: + { + HeapTuple stxTup; + Form_pg_statistic_ext stxForm; + + stxTup = SearchSysCache1(STATEXTOID, + ObjectIdGetDatum(object->objectId)); + if (!HeapTupleIsValid(stxTup)) + elog(ERROR, "could not find tuple for statistics object %u", + object->objectId); + + stxForm = (Form_pg_statistic_ext) GETSTRUCT(stxTup); + + appendStringInfo(&buffer, _("statistics object %s"), + NameStr(stxForm->stxname)); + + ReleaseSysCache(stxTup); + break; + } + case OCLASS_TSPARSER: { HeapTuple tup; @@ -3365,21 +3359,6 @@ getObjectDescription(const ObjectAddress *object) break; } - case OCLASS_AM: - { - HeapTuple tup; - - tup = SearchSysCache1(AMOID, - ObjectIdGetDatum(object->objectId)); - if (!HeapTupleIsValid(tup)) - elog(ERROR, "cache lookup failed for access method %u", - object->objectId); - appendStringInfo(&buffer, _("access method %s"), - NameStr(((Form_pg_am) GETSTRUCT(tup))->amname)); - ReleaseSysCache(tup); - break; - } - case OCLASS_PUBLICATION: { appendStringInfo(&buffer, _("publication %s"), @@ -3414,6 +3393,32 @@ getObjectDescription(const ObjectAddress *object) get_subscription_name(object->objectId)); break; } + + case OCLASS_TRANSFORM: + { + HeapTuple trfTup; + Form_pg_transform trfForm; + + trfTup = SearchSysCache1(TRFOID, + ObjectIdGetDatum(object->objectId)); + if (!HeapTupleIsValid(trfTup)) + elog(ERROR, "could not find tuple for transform %u", + object->objectId); + + trfForm = (Form_pg_transform) GETSTRUCT(trfTup); + + appendStringInfo(&buffer, _("transform for %s language %s"), + format_type_be(trfForm->trftype), + get_language_name(trfForm->trflang, false)); + + ReleaseSysCache(trfTup); + break; + } + + /* + * There's intentionally no default: case here; we want the + * compiler to warn if a new OCLASS hasn't been handled above. + */ } return buffer.data; @@ -3810,6 +3815,10 @@ getObjectTypeDescription(const ObjectAddress *object) appendStringInfoString(&buffer, "operator family"); break; + case OCLASS_AM: + appendStringInfoString(&buffer, "access method"); + break; + case OCLASS_AMOP: appendStringInfoString(&buffer, "operator of access method"); break; @@ -3830,6 +3839,10 @@ getObjectTypeDescription(const ObjectAddress *object) appendStringInfoString(&buffer, "schema"); break; + case OCLASS_STATISTIC_EXT: + appendStringInfoString(&buffer, "statistics object"); + break; + case OCLASS_TSPARSER: appendStringInfoString(&buffer, "text search parser"); break; @@ -3886,14 +3899,6 @@ getObjectTypeDescription(const ObjectAddress *object) appendStringInfoString(&buffer, "policy"); break; - case OCLASS_TRANSFORM: - appendStringInfoString(&buffer, "transform"); - break; - - case OCLASS_AM: - appendStringInfoString(&buffer, "access method"); - break; - case OCLASS_PUBLICATION: appendStringInfoString(&buffer, "publication"); break; @@ -3906,14 +3911,14 @@ getObjectTypeDescription(const ObjectAddress *object) appendStringInfoString(&buffer, "subscription"); break; - case OCLASS_STATISTIC_EXT: - appendStringInfoString(&buffer, "statistics object"); + case OCLASS_TRANSFORM: + appendStringInfoString(&buffer, "transform"); break; - default: - appendStringInfo(&buffer, "unrecognized object class %u", - object->classId); - break; + /* + * There's intentionally no default: case here; we want the + * compiler to warn if a new OCLASS hasn't been handled above. + */ } return buffer.data; @@ -4329,6 +4334,20 @@ getObjectIdentityParts(const ObjectAddress *object, getOpFamilyIdentity(&buffer, object->objectId, objname); break; + case OCLASS_AM: + { + char *amname; + + amname = get_am_name(object->objectId); + if (!amname) + elog(ERROR, "cache lookup failed for access method %u", + object->objectId); + appendStringInfoString(&buffer, quote_identifier(amname)); + if (objname) + *objname = list_make1(amname); + } + break; + case OCLASS_AMOP: { Relation amopDesc; @@ -4489,32 +4508,6 @@ getObjectIdentityParts(const ObjectAddress *object, break; } - case OCLASS_POLICY: - { - Relation polDesc; - HeapTuple tup; - Form_pg_policy policy; - - polDesc = heap_open(PolicyRelationId, AccessShareLock); - - tup = get_catalog_object_by_oid(polDesc, object->objectId); - - if (!HeapTupleIsValid(tup)) - elog(ERROR, "could not find tuple for policy %u", - object->objectId); - - policy = (Form_pg_policy) GETSTRUCT(tup); - - appendStringInfo(&buffer, "%s on ", - quote_identifier(NameStr(policy->polname))); - getRelationIdentity(&buffer, policy->polrelid, objname); - if (objname) - *objname = lappend(*objname, pstrdup(NameStr(policy->polname))); - - heap_close(polDesc, AccessShareLock); - break; - } - case OCLASS_SCHEMA: { char *nspname; @@ -4530,6 +4523,29 @@ getObjectIdentityParts(const ObjectAddress *object, break; } + case OCLASS_STATISTIC_EXT: + { + HeapTuple tup; + Form_pg_statistic_ext formStatistic; + char *schema; + + tup = SearchSysCache1(STATEXTOID, + ObjectIdGetDatum(object->objectId)); + if (!HeapTupleIsValid(tup)) + elog(ERROR, "cache lookup failed for statistics object %u", + object->objectId); + formStatistic = (Form_pg_statistic_ext) GETSTRUCT(tup); + schema = get_namespace_name_or_temp(formStatistic->stxnamespace); + appendStringInfoString(&buffer, + quote_qualified_identifier(schema, + NameStr(formStatistic->stxname))); + if (objname) + *objname = list_make2(schema, + pstrdup(NameStr(formStatistic->stxname))); + ReleaseSysCache(tup); + } + break; + case OCLASS_TSPARSER: { HeapTuple tup; @@ -4838,53 +4854,31 @@ getObjectIdentityParts(const ObjectAddress *object, break; } - case OCLASS_TRANSFORM: + case OCLASS_POLICY: { - Relation transformDesc; + Relation polDesc; HeapTuple tup; - Form_pg_transform transform; - char *transformLang; - char *transformType; + Form_pg_policy policy; - transformDesc = heap_open(TransformRelationId, AccessShareLock); + polDesc = heap_open(PolicyRelationId, AccessShareLock); - tup = get_catalog_object_by_oid(transformDesc, object->objectId); + tup = get_catalog_object_by_oid(polDesc, object->objectId); if (!HeapTupleIsValid(tup)) - elog(ERROR, "could not find tuple for transform %u", + elog(ERROR, "could not find tuple for policy %u", object->objectId); - transform = (Form_pg_transform) GETSTRUCT(tup); - - transformType = format_type_be_qualified(transform->trftype); - transformLang = get_language_name(transform->trflang, false); + policy = (Form_pg_policy) GETSTRUCT(tup); - appendStringInfo(&buffer, "for %s on language %s", - transformType, - transformLang); + appendStringInfo(&buffer, "%s on ", + quote_identifier(NameStr(policy->polname))); + getRelationIdentity(&buffer, policy->polrelid, objname); if (objname) - { - *objname = list_make1(transformType); - *objargs = list_make1(pstrdup(transformLang)); - } - - heap_close(transformDesc, AccessShareLock); - } - break; - - case OCLASS_AM: - { - char *amname; + *objname = lappend(*objname, pstrdup(NameStr(policy->polname))); - amname = get_am_name(object->objectId); - if (!amname) - elog(ERROR, "cache lookup failed for access method %u", - object->objectId); - appendStringInfoString(&buffer, quote_identifier(amname)); - if (objname) - *objname = list_make1(amname); + heap_close(polDesc, AccessShareLock); + break; } - break; case OCLASS_PUBLICATION: { @@ -4938,35 +4932,44 @@ getObjectIdentityParts(const ObjectAddress *object, break; } - case OCLASS_STATISTIC_EXT: + case OCLASS_TRANSFORM: { + Relation transformDesc; HeapTuple tup; - Form_pg_statistic_ext formStatistic; - char *schema; + Form_pg_transform transform; + char *transformLang; + char *transformType; + + transformDesc = heap_open(TransformRelationId, AccessShareLock); + + tup = get_catalog_object_by_oid(transformDesc, object->objectId); - tup = SearchSysCache1(STATEXTOID, - ObjectIdGetDatum(object->objectId)); if (!HeapTupleIsValid(tup)) - elog(ERROR, "cache lookup failed for statistics object %u", + elog(ERROR, "could not find tuple for transform %u", object->objectId); - formStatistic = (Form_pg_statistic_ext) GETSTRUCT(tup); - schema = get_namespace_name_or_temp(formStatistic->stxnamespace); - appendStringInfoString(&buffer, - quote_qualified_identifier(schema, - NameStr(formStatistic->stxname))); + + transform = (Form_pg_transform) GETSTRUCT(tup); + + transformType = format_type_be_qualified(transform->trftype); + transformLang = get_language_name(transform->trflang, false); + + appendStringInfo(&buffer, "for %s on language %s", + transformType, + transformLang); if (objname) - *objname = list_make2(schema, - pstrdup(NameStr(formStatistic->stxname))); - ReleaseSysCache(tup); + { + *objname = list_make1(transformType); + *objargs = list_make1(pstrdup(transformLang)); + } + + heap_close(transformDesc, AccessShareLock); } break; - default: - appendStringInfo(&buffer, "unrecognized object %u %u %d", - object->classId, - object->objectId, - object->objectSubId); - break; + /* + * There's intentionally no default: case here; we want the + * compiler to warn if a new OCLASS hasn't been handled above. + */ } /* diff --git a/src/backend/commands/alter.c b/src/backend/commands/alter.c index bc897e409c..a4b949d8c7 100644 --- a/src/backend/commands/alter.c +++ b/src/backend/commands/alter.c @@ -543,7 +543,8 @@ ExecAlterObjectSchemaStmt(AlterObjectSchemaStmt *stmt, * so it only needs to cover object types that can be members of an * extension, and it doesn't have to deal with certain special cases * such as not wanting to process array types --- those should never - * be direct members of an extension anyway. + * be direct members of an extension anyway. Nonetheless, we insist + * on listing all OCLASS types in the switch. * * Returns the OID of the object's previous namespace, or InvalidOid if * object doesn't have a schema. @@ -578,12 +579,13 @@ AlterObjectNamespace_oid(Oid classId, Oid objid, Oid nspOid, oldNspOid = AlterTypeNamespace_oid(objid, nspOid, objsMoved); break; + case OCLASS_PROC: case OCLASS_COLLATION: case OCLASS_CONVERSION: case OCLASS_OPERATOR: case OCLASS_OPCLASS: case OCLASS_OPFAMILY: - case OCLASS_PROC: + case OCLASS_STATISTIC_EXT: case OCLASS_TSPARSER: case OCLASS_TSDICT: case OCLASS_TSTEMPLATE: @@ -600,8 +602,38 @@ AlterObjectNamespace_oid(Oid classId, Oid objid, Oid nspOid, } break; - default: + case OCLASS_CAST: + case OCLASS_CONSTRAINT: + case OCLASS_DEFAULT: + case OCLASS_LANGUAGE: + case OCLASS_LARGEOBJECT: + case OCLASS_AM: + case OCLASS_AMOP: + case OCLASS_AMPROC: + case OCLASS_REWRITE: + case OCLASS_TRIGGER: + case OCLASS_SCHEMA: + case OCLASS_ROLE: + case OCLASS_DATABASE: + case OCLASS_TBLSPACE: + case OCLASS_FDW: + case OCLASS_FOREIGN_SERVER: + case OCLASS_USER_MAPPING: + case OCLASS_DEFACL: + case OCLASS_EXTENSION: + case OCLASS_EVENT_TRIGGER: + case OCLASS_POLICY: + case OCLASS_PUBLICATION: + case OCLASS_PUBLICATION_REL: + case OCLASS_SUBSCRIPTION: + case OCLASS_TRANSFORM: + /* ignore object types that don't have schema-qualified names */ break; + + /* + * There's intentionally no default: case here; we want the + * compiler to warn if a new OCLASS hasn't been handled above. + */ } return oldNspOid; diff --git a/src/backend/commands/event_trigger.c b/src/backend/commands/event_trigger.c index d7c199f314..d1983257c2 100644 --- a/src/backend/commands/event_trigger.c +++ b/src/backend/commands/event_trigger.c @@ -1122,8 +1122,15 @@ EventTriggerSupportsObjectType(ObjectType obtype) case OBJECT_USER_MAPPING: case OBJECT_VIEW: return true; + + /* + * There's intentionally no default: case here; we want the + * compiler to warn if a new ObjectType hasn't been handled above. + */ } - return true; + + /* Shouldn't get here, but if we do, say "no support" */ + return false; } /* @@ -1155,12 +1162,13 @@ EventTriggerSupportsObjectClass(ObjectClass objclass) case OCLASS_OPERATOR: case OCLASS_OPCLASS: case OCLASS_OPFAMILY: + case OCLASS_AM: case OCLASS_AMOP: case OCLASS_AMPROC: case OCLASS_REWRITE: case OCLASS_TRIGGER: case OCLASS_SCHEMA: - case OCLASS_TRANSFORM: + case OCLASS_STATISTIC_EXT: case OCLASS_TSPARSER: case OCLASS_TSDICT: case OCLASS_TSTEMPLATE: @@ -1171,15 +1179,20 @@ EventTriggerSupportsObjectClass(ObjectClass objclass) case OCLASS_DEFACL: case OCLASS_EXTENSION: case OCLASS_POLICY: - case OCLASS_AM: case OCLASS_PUBLICATION: case OCLASS_PUBLICATION_REL: case OCLASS_SUBSCRIPTION: - case OCLASS_STATISTIC_EXT: + case OCLASS_TRANSFORM: return true; + + /* + * There's intentionally no default: case here; we want the + * compiler to warn if a new OCLASS hasn't been handled above. + */ } - return true; + /* Shouldn't get here, but if we do, say "no support" */ + return false; } bool @@ -1204,10 +1217,15 @@ EventTriggerSupportsGrantObjectType(GrantObjectType objtype) case ACL_OBJECT_NAMESPACE: case ACL_OBJECT_TYPE: return true; - default: - Assert(false); - return true; + + /* + * There's intentionally no default: case here; we want the + * compiler to warn if a new ACL class hasn't been handled above. + */ } + + /* Shouldn't get here, but if we do, say "no support" */ + return false; } /* @@ -2229,35 +2247,50 @@ stringify_grantobjtype(GrantObjectType objtype) return "TABLESPACE"; case ACL_OBJECT_TYPE: return "TYPE"; - default: - elog(ERROR, "unrecognized type %d", objtype); - return "???"; /* keep compiler quiet */ } + + elog(ERROR, "unrecognized grant object type: %d", (int) objtype); + return "???"; /* keep compiler quiet */ } /* * Return the GrantObjectType as a string; as above, but use the spelling - * in ALTER DEFAULT PRIVILEGES commands instead. + * in ALTER DEFAULT PRIVILEGES commands instead. Generally this is just + * the plural. */ static const char * stringify_adefprivs_objtype(GrantObjectType objtype) { switch (objtype) { + case ACL_OBJECT_COLUMN: + return "COLUMNS"; case ACL_OBJECT_RELATION: return "TABLES"; - break; - case ACL_OBJECT_FUNCTION: - return "FUNCTIONS"; - break; case ACL_OBJECT_SEQUENCE: return "SEQUENCES"; - break; + case ACL_OBJECT_DATABASE: + return "DATABASES"; + case ACL_OBJECT_DOMAIN: + return "DOMAINS"; + case ACL_OBJECT_FDW: + return "FOREIGN DATA WRAPPERS"; + case ACL_OBJECT_FOREIGN_SERVER: + return "FOREIGN SERVERS"; + case ACL_OBJECT_FUNCTION: + return "FUNCTIONS"; + case ACL_OBJECT_LANGUAGE: + return "LANGUAGES"; + case ACL_OBJECT_LARGEOBJECT: + return "LARGE OBJECTS"; + case ACL_OBJECT_NAMESPACE: + return "SCHEMAS"; + case ACL_OBJECT_TABLESPACE: + return "TABLESPACES"; case ACL_OBJECT_TYPE: return "TYPES"; - break; - default: - elog(ERROR, "unrecognized type %d", objtype); - return "???"; /* keep compiler quiet */ } + + elog(ERROR, "unrecognized grant object type: %d", (int) objtype); + return "???"; /* keep compiler quiet */ }