Use new errdetail_log() mechanism to provide a less klugy way of reporting
authorTom Lane
Mon, 24 Mar 2008 19:47:35 +0000 (19:47 +0000)
committerTom Lane
Mon, 24 Mar 2008 19:47:35 +0000 (19:47 +0000)
large numbers of dependencies on a role that couldn't be dropped.
Per a comment from Alvaro.

src/backend/catalog/pg_shdepend.c
src/backend/commands/user.c
src/include/catalog/dependency.h

index 5e999f5816881d2448d689c0a933c45165f2e403..9e51f56f271f111e47af256a3658d0f99cf78003 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/catalog/pg_shdepend.c,v 1.24 2008/03/24 19:12:49 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/catalog/pg_shdepend.c,v 1.25 2008/03/24 19:47:35 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -454,11 +454,14 @@ typedef struct
  * checkSharedDependencies
  *
  * Check whether there are shared dependency entries for a given shared
- * object. Returns a string containing a newline-separated list of object
+ * object; return true if so.
+ *
+ * In addition, return a string containing a newline-separated list of object
  * descriptions that depend on the shared object, or NULL if none is found.
- * The size of the returned string is limited to about MAX_REPORTED_DEPS lines;
- * if there are more objects than that, the output is returned truncated at
- * that point while the full message is logged to the postmaster log.
+ * We actually return two such strings; the "detail" result is suitable for
+ * returning to the client as an errdetail() string, and is limited in size.
+ * The "detail_log" string is potentially much longer, and should be emitted
+ * to the server log only.
  *
  * We can find three different kinds of dependencies: dependencies on objects
  * of the current database; dependencies on shared objects; and dependencies
@@ -468,8 +471,9 @@ typedef struct
  *
  * If we find a SHARED_DEPENDENCY_PIN entry, we can error out early.
  */
-char *
-checkSharedDependencies(Oid classId, Oid objectId)
+bool
+checkSharedDependencies(Oid classId, Oid objectId,
+                       char **detail_msg, char **detail_log_msg)
 {
    Relation    sdepRel;
    ScanKeyData key[2];
@@ -487,9 +491,7 @@ checkSharedDependencies(Oid classId, Oid objectId)
    /*
     * We limit the number of dependencies reported to the client to
     * MAX_REPORTED_DEPS, since client software may not deal well with
-    * enormous error strings.  The server log always gets a full report,
-    * which is collected in a separate StringInfo if and only if we detect
-    * that the client report is going to be truncated.
+    * enormous error strings.  The server log always gets a full report.
     */
 #define MAX_REPORTED_DEPS 100
 
@@ -546,15 +548,9 @@ checkSharedDependencies(Oid classId, Oid objectId)
                                       sdepForm->deptype, 0);
            }
            else
-           {
                numNotReportedDeps++;
-               /* initialize the server-only log line */
-               if (alldescs.len == 0)
-                   appendBinaryStringInfo(&alldescs, descs.data, descs.len);
-
-               storeObjectDescription(&alldescs, LOCAL_OBJECT, &object,
-                                      sdepForm->deptype, 0);
-           }
+           storeObjectDescription(&alldescs, LOCAL_OBJECT, &object,
+                                  sdepForm->deptype, 0);
        }
        else if (sdepForm->dbid == InvalidOid)
        {
@@ -565,15 +561,9 @@ checkSharedDependencies(Oid classId, Oid objectId)
                                       sdepForm->deptype, 0);
            }
            else
-           {
                numNotReportedDeps++;
-               /* initialize the server-only log line */
-               if (alldescs.len == 0)
-                   appendBinaryStringInfo(&alldescs, descs.data, descs.len);
-
-               storeObjectDescription(&alldescs, SHARED_OBJECT, &object,
-                                      sdepForm->deptype, 0);
-           }
+           storeObjectDescription(&alldescs, SHARED_OBJECT, &object,
+                                  sdepForm->deptype, 0);
        }
        else
        {
@@ -612,9 +602,7 @@ checkSharedDependencies(Oid classId, Oid objectId)
    heap_close(sdepRel, AccessShareLock);
 
    /*
-    * Report dependencies on remote databases.  If we're truncating the
-    * output already, don't put a line per database, but a single one for all
-    * of them.  Otherwise add as many as fit in MAX_REPORTED_DEPS.
+    * Summarize dependencies in remote databases.
     */
    foreach(cell, remDeps)
    {
@@ -631,15 +619,9 @@ checkSharedDependencies(Oid classId, Oid objectId)
                                   SHARED_DEPENDENCY_INVALID, dep->count);
        }
        else
-       {
            numNotReportedDbs++;
-           /* initialize the server-only log line */
-           if (alldescs.len == 0)
-               appendBinaryStringInfo(&alldescs, descs.data, descs.len);
-
-           storeObjectDescription(&alldescs, REMOTE_OBJECT, &object,
-                                  SHARED_DEPENDENCY_INVALID, dep->count);
-       }
+       storeObjectDescription(&alldescs, REMOTE_OBJECT, &object,
+                              SHARED_DEPENDENCY_INVALID, dep->count);
    }
 
    list_free_deep(remDeps);
@@ -648,7 +630,8 @@ checkSharedDependencies(Oid classId, Oid objectId)
    {
        pfree(descs.data);
        pfree(alldescs.data);
-       return NULL;
+       *detail_msg = *detail_log_msg = NULL;
+       return false;
    }
 
    if (numNotReportedDeps > 0)
@@ -660,22 +643,9 @@ checkSharedDependencies(Oid classId, Oid objectId)
                                   "(see server log for list)"),
                         numNotReportedDbs);
 
-   if (numNotReportedDeps > 0 || numNotReportedDbs > 0)
-   {
-       ObjectAddress obj;
-
-       obj.classId = classId;
-       obj.objectId = objectId;
-       obj.objectSubId = 0;
-       ereport(LOG,
-               (errmsg("there are objects dependent on %s",
-                       getObjectDescription(&obj)),
-                errdetail("%s", alldescs.data)));
-   }
-
-   pfree(alldescs.data);
-
-   return descs.data;
+   *detail_msg = descs.data;
+   *detail_log_msg = alldescs.data;
+   return true;
 }
 
 /*
index 64e5cd8f0b856d6cbd8e24a3d538a1853d448ad8..2b790227f32d38b9f26a093e4f69c1bcd1835a03 100644 (file)
@@ -6,7 +6,7 @@
  * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/backend/commands/user.c,v 1.178 2008/01/01 19:45:49 momjian Exp $
+ * $PostgreSQL: pgsql/src/backend/commands/user.c,v 1.179 2008/03/24 19:47:35 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -828,6 +828,7 @@ DropRole(DropRoleStmt *stmt)
                    tmp_tuple;
        ScanKeyData scankey;
        char       *detail;
+       char       *detail_log;
        SysScanDesc sscan;
        Oid         roleid;
 
@@ -885,12 +886,14 @@ DropRole(DropRoleStmt *stmt)
        LockSharedObject(AuthIdRelationId, roleid, 0, AccessExclusiveLock);
 
        /* Check for pg_shdepend entries depending on this role */
-       if ((detail = checkSharedDependencies(AuthIdRelationId, roleid)) != NULL)
+       if (checkSharedDependencies(AuthIdRelationId, roleid,
+                                   &detail, &detail_log))
            ereport(ERROR,
                    (errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST),
                     errmsg("role \"%s\" cannot be dropped because some objects depend on it",
                            role),
-                    errdetail("%s", detail)));
+                    errdetail("%s", detail),
+                    errdetail_log("%s", detail_log)));
 
        /*
         * Remove the role from the pg_authid table
index d288cf79ed429046b174091a2fb23d72b449557d..572dedbc4fe0b6c9d3124fb1864bd064cd06e392 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/catalog/dependency.h,v 1.33 2008/01/01 19:45:56 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/catalog/dependency.h,v 1.34 2008/03/24 19:47:35 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -229,7 +229,8 @@ extern void updateAclDependencies(Oid classId, Oid objectId,
                      int noldmembers, Oid *oldmembers,
                      int nnewmembers, Oid *newmembers);
 
-extern char *checkSharedDependencies(Oid classId, Oid objectId);
+extern bool checkSharedDependencies(Oid classId, Oid objectId,
+                                   char **detail_msg, char **detail_log_msg);
 
 extern void copyTemplateDependencies(Oid templateDbId, Oid newDbId);