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 cdf453562fc6d0bc04a24ef87898c700ac2bc7b6..806db7f35ea336e9397364ab6e16e9bbbd8af883 100644 (file)
--- 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 e3efb98b1d09f241ad5068f0d5887a5482342663..3dfb8fa4f9d1d2653e48384c167c6d4b10dda3c9 100644 (file)
--- 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 bc897e409c006cf46aa3b39367ef788b538f2496..a4b949d8c712cce63201ebc169e9e98265682c4a 100644 (file)
--- 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 d7c199f3144288d786d06922b1c0b259e6eb469c..d1983257c2fc723aefa585f5d947cc2b6e866b6d 100644 (file)
--- 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 */
 }