Avoid sometimes printing both tables and their columns in DROP CASCADE.

tglsfdc · tglsfdc · commit 9194c4270b28 · 2019-01-18T11:05:11.000-05:00
A cascaded drop might find independent reasons to drop both a table and some column of the table (for instance, a schema drop might include dropping a data type used in some table in the schema). Depending on the order of visitation of pg_depend entries, we might report the table column and the whole table as separate objects-to-be-dropped, or we might only report the table. This is confusing and leads to unstable regression test output, so fix it to report only the table regardless of visitation order. Per gripe from Peter Geoghegan. This is just cosmetic from a user's standpoint, and we haven't actually seen regression test problems in practice (yet), so I'll refrain from back-patching. Discussion: https://postgr.es/m/15908.1547762076@sss.pgh.pa.us
diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
@@ -102,6 +102,7 @@ typedef struct
 #define DEPFLAG_INTERNAL	0x0008	/* reached via internal dependency */
 #define DEPFLAG_EXTENSION	0x0010	/* reached via extension dependency */
 #define DEPFLAG_REVERSE		0x0020	/* reverse internal/extension link */
+#define DEPFLAG_SUBOBJECT	0x0040	/* subobject of another deletable object */
 
 
 /* expansible list of ObjectAddresses */
@@ -886,6 +887,10 @@ reportDependentObjects(const ObjectAddresses *targetObjects,
 		if (extra->flags & DEPFLAG_ORIGINAL)
 			continue;
 
+		/* Also ignore sub-objects; we'll report the whole object elsewhere */
+		if (extra->flags & DEPFLAG_SUBOBJECT)
+			continue;
+
 		objDesc = getObjectDescription(obj);
 
 		/*
@@ -2320,13 +2325,19 @@ object_address_present_add_flags(const ObjectAddress *object,
 				 * DROP COLUMN action even though we know we're gonna delete
 				 * the table later.
 				 *
+				 * What we can do, though, is mark this as a subobject so that
+				 * we don't report it separately, which is confusing because
+				 * it's unpredictable whether it happens or not.  But do so
+				 * only if flags != 0 (flags == 0 is a read-only probe).
+				 *
 				 * Because there could be other subobjects of this object in
 				 * the array, this case means we always have to loop through
 				 * the whole array; we cannot exit early on a match.
 				 */
 				ObjectAddressExtra *thisextra = addrs->extras + i;
 
-				thisextra->flags |= flags;
+				if (flags)
+					thisextra->flags |= (flags | DEPFLAG_SUBOBJECT);
 			}
 		}
 	}
@@ -2374,7 +2385,8 @@ stack_address_present_add_flags(const ObjectAddress *object,
 				 * object_address_present_add_flags(), we should propagate
 				 * flags for the whole object to each of its subobjects.
 				 */
-				stackptr->flags |= flags;
+				if (flags)
+					stackptr->flags |= (flags | DEPFLAG_SUBOBJECT);
 			}
 		}
 	}
