Rewrite DROP's dependency traversal algorithm into an honest two-pass

algorithm, replacing the original intention of a one-pass search, which
had been hacked up over time to be partially two-pass in hopes of handling
various corner cases better.  It still wasn't quite there, especially as
regards emitting unwanted NOTICE messages.  More importantly, this approach
lets us fix a number of open bugs concerning concurrent DROP scenarios,
because we can take locks during the first pass and avoid traversing to
dependent objects that were just deleted by someone else.

There is more that can be done here, but I'll go ahead and commit the
base patch before working on the options.

src/backend/access/index/genam.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/index/genam.c,v 1.68 2008/04/13 19:18:14 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/index/genam.c,v 1.69 2008/06/08 22:41:04 tgl Exp $
  *
  * NOTES
  *	  many of the old access method routines have been turned into
@@ -251,6 +251,47 @@ systable_getnext(SysScanDesc sysscan)
 	return htup;
 }
 
+/*
+ * systable_recheck_tuple --- recheck visibility of most-recently-fetched tuple
+ *
+ * This is useful to test whether an object was deleted while we waited to
+ * acquire lock on it.
+ *
+ * Note: we don't actually *need* the tuple to be passed in, but it's a
+ * good crosscheck that the caller is interested in the right tuple.
+ */
+bool
+systable_recheck_tuple(SysScanDesc sysscan, HeapTuple tup)
+{
+	bool		result;
+
+	if (sysscan->irel)
+	{
+		IndexScanDesc scan = sysscan->iscan;
+
+		Assert(tup == &scan->xs_ctup);
+		Assert(BufferIsValid(scan->xs_cbuf));
+		/* must hold a buffer lock to call HeapTupleSatisfiesVisibility */
+		LockBuffer(scan->xs_cbuf, BUFFER_LOCK_SHARE);
+		result = HeapTupleSatisfiesVisibility(tup, scan->xs_snapshot,
+											  scan->xs_cbuf);
+		LockBuffer(scan->xs_cbuf, BUFFER_LOCK_UNLOCK);
+	}
+	else
+	{
+		HeapScanDesc scan = sysscan->scan;
+
+		Assert(tup == &scan->rs_ctup);
+		Assert(BufferIsValid(scan->rs_cbuf));
+		/* must hold a buffer lock to call HeapTupleSatisfiesVisibility */
+		LockBuffer(scan->rs_cbuf, BUFFER_LOCK_SHARE);
+		result = HeapTupleSatisfiesVisibility(tup, scan->rs_snapshot,
+											  scan->rs_cbuf);
+		LockBuffer(scan->rs_cbuf, BUFFER_LOCK_UNLOCK);
+	}
+	return result;
+}
+
 /*
  * systable_endscan --- close scan, release resources
  *

src/include/access/genam.h

@@ -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/access/genam.h,v 1.72 2008/05/12 00:00:53 alvherre Exp $
+ * $PostgreSQL: pgsql/src/include/access/genam.h,v 1.73 2008/06/08 22:41:04 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -138,6 +138,7 @@ extern SysScanDesc systable_beginscan(Relation heapRelation,
 				   Snapshot snapshot,
 				   int nkeys, ScanKey key);
 extern HeapTuple systable_getnext(SysScanDesc sysscan);
+extern bool systable_recheck_tuple(SysScanDesc sysscan, HeapTuple tup);
 extern void systable_endscan(SysScanDesc sysscan);
 extern SysScanDesc systable_beginscan_ordered(Relation heapRelation,
 											  Relation indexRelation,

src/include/catalog/dependency.h

@@ -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.35 2008/05/16 23:36:05 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/catalog/dependency.h,v 1.36 2008/06/08 22:41:04 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -17,7 +17,7 @@
 #include "nodes/parsenodes.h"	/* for DropBehavior */
 
 
-/*----------
+/*
  * Precise semantics of a dependency relationship are specified by the
  * DependencyType code (which is stored in a "char" field in pg_depend,
  * so we assign ASCII-code values to the enumeration members).
@@ -56,7 +56,6 @@
  * contain zeroes.
  *
  * Other dependency flavors may be needed in future.
- *----------
  */
 
 typedef enum DependencyType
@@ -178,7 +177,7 @@ extern void add_exact_object_address(const ObjectAddress *object,
 						 ObjectAddresses *addrs);
 
 extern bool object_address_present(const ObjectAddress *object,
-					   ObjectAddresses *addrs);
+					   const ObjectAddresses *addrs);
 
 extern void record_object_address_dependencies(const ObjectAddress *depender,
 								   ObjectAddresses *referenced,

src/test/regress/expected/alter_table.out

@@ -376,7 +376,6 @@ select test2 from atacc2;
 
 drop table atacc2 cascade;
 NOTICE:  drop cascades to table atacc3
-NOTICE:  drop cascades to constraint foo on table atacc3
 drop table atacc1;
 -- adding only to a parent is disallowed as of 8.4
 create table atacc1 (test int);
@@ -1251,7 +1250,6 @@ select * from p1;
 
 drop table p1 cascade;
 NOTICE:  drop cascades to table c1
-NOTICE:  drop cascades to constraint p1_a1_check on table c1
 -- test that operations with a dropped column do not try to reference
 -- its datatype
 create domain mytype as text;
@@ -1470,12 +1468,8 @@ select alter2.plus1(41);
 
 -- clean up
 drop schema alter2 cascade;
-NOTICE:  drop cascades to type alter2.ctype
-NOTICE:  drop cascades to type alter2.posint
-NOTICE:  drop cascades to function alter2.plus1(integer)
-NOTICE:  drop cascades to view alter2.v1
-NOTICE:  drop cascades to rule _RETURN on view alter2.v1
-NOTICE:  drop cascades to sequence alter2.t1_f1_seq
-NOTICE:  drop cascades to default for table alter2.t1 column f1
 NOTICE:  drop cascades to table alter2.t1
-NOTICE:  drop cascades to constraint t1_f2_check on table alter2.t1
+NOTICE:  drop cascades to view alter2.v1
+NOTICE:  drop cascades to function alter2.plus1(integer)
+NOTICE:  drop cascades to type alter2.posint
+NOTICE:  drop cascades to type alter2.ctype

src/test/regress/expected/create_view.out

@@ -237,72 +237,43 @@ And relnamespace IN (SELECT OID FROM pg_namespace WHERE nspname LIKE 'pg_temp%')
 (1 row)
 
 DROP SCHEMA temp_view_test CASCADE;
-NOTICE:  drop cascades to view temp_view_test.v9
-NOTICE:  drop cascades to rule _RETURN on view temp_view_test.v9
-NOTICE:  drop cascades to sequence temp_view_test.seq1
-NOTICE:  drop cascades to view temp_view_test.v8
-NOTICE:  drop cascades to rule _RETURN on view temp_view_test.v8
-NOTICE:  drop cascades to view temp_view_test.v7
-NOTICE:  drop cascades to rule _RETURN on view temp_view_test.v7
-NOTICE:  drop cascades to view temp_view_test.v6
-NOTICE:  drop cascades to rule _RETURN on view temp_view_test.v6
-NOTICE:  drop cascades to view temp_view_test.v5
-NOTICE:  drop cascades to rule _RETURN on view temp_view_test.v5
-NOTICE:  drop cascades to view temp_view_test.v4
-NOTICE:  drop cascades to rule _RETURN on view temp_view_test.v4
-NOTICE:  drop cascades to view temp_view_test.v3
-NOTICE:  drop cascades to rule _RETURN on view temp_view_test.v3
-NOTICE:  drop cascades to view temp_view_test.v2
-NOTICE:  drop cascades to rule _RETURN on view temp_view_test.v2
-NOTICE:  drop cascades to view temp_view_test.v1
-NOTICE:  drop cascades to rule _RETURN on view temp_view_test.v1
-NOTICE:  drop cascades to table temp_view_test.base_table2
-NOTICE:  drop cascades to rule _RETURN on view v5_temp
-NOTICE:  drop cascades to view v5_temp
 NOTICE:  drop cascades to table temp_view_test.base_table
-NOTICE:  drop cascades to rule _RETURN on view v9_temp
-NOTICE:  drop cascades to view v9_temp
-NOTICE:  drop cascades to rule _RETURN on view v8_temp
-NOTICE:  drop cascades to view v8_temp
-NOTICE:  drop cascades to rule _RETURN on view v6_temp
-NOTICE:  drop cascades to view v6_temp
-NOTICE:  drop cascades to rule _RETURN on view v4_temp
-NOTICE:  drop cascades to view v4_temp
-NOTICE:  drop cascades to rule _RETURN on view v2_temp
-NOTICE:  drop cascades to view v2_temp
-NOTICE:  drop cascades to rule _RETURN on view v11_temp
-NOTICE:  drop cascades to view v11_temp
-NOTICE:  drop cascades to rule _RETURN on view v12_temp
-NOTICE:  drop cascades to view v12_temp
-NOTICE:  drop cascades to rule _RETURN on view v7_temp
 NOTICE:  drop cascades to view v7_temp
-NOTICE:  drop cascades to rule _RETURN on view v10_temp
 NOTICE:  drop cascades to view v10_temp
+NOTICE:  drop cascades to view v11_temp
+NOTICE:  drop cascades to view v12_temp
+NOTICE:  drop cascades to view v2_temp
+NOTICE:  drop cascades to view v4_temp
+NOTICE:  drop cascades to view v6_temp
+NOTICE:  drop cascades to view v8_temp
+NOTICE:  drop cascades to view v9_temp
+NOTICE:  drop cascades to table temp_view_test.base_table2
+NOTICE:  drop cascades to view v5_temp
+NOTICE:  drop cascades to view temp_view_test.v1
+NOTICE:  drop cascades to view temp_view_test.v2
+NOTICE:  drop cascades to view temp_view_test.v3
+NOTICE:  drop cascades to view temp_view_test.v4
+NOTICE:  drop cascades to view temp_view_test.v5
+NOTICE:  drop cascades to view temp_view_test.v6
+NOTICE:  drop cascades to view temp_view_test.v7
+NOTICE:  drop cascades to view temp_view_test.v8
+NOTICE:  drop cascades to sequence temp_view_test.seq1
+NOTICE:  drop cascades to view temp_view_test.v9
 DROP SCHEMA testviewschm2 CASCADE;
-NOTICE:  drop cascades to view pubview
-NOTICE:  drop cascades to rule _RETURN on view pubview
-NOTICE:  drop cascades to table tbl4
-NOTICE:  drop cascades to rule _RETURN on view mytempview
-NOTICE:  drop cascades to view mytempview
-NOTICE:  drop cascades to table tbl3
-NOTICE:  drop cascades to table tbl2
-NOTICE:  drop cascades to table tbl1
-NOTICE:  drop cascades to view nontemp4
-NOTICE:  drop cascades to rule _RETURN on view nontemp4
-NOTICE:  drop cascades to view nontemp3
-NOTICE:  drop cascades to rule _RETURN on view nontemp3
-NOTICE:  drop cascades to view nontemp2
-NOTICE:  drop cascades to rule _RETURN on view nontemp2
-NOTICE:  drop cascades to view nontemp1
-NOTICE:  drop cascades to rule _RETURN on view nontemp1
-NOTICE:  drop cascades to table t2
 NOTICE:  drop cascades to table t1
-NOTICE:  drop cascades to rule _RETURN on view temporal4
-NOTICE:  drop cascades to view temporal4
-NOTICE:  drop cascades to rule _RETURN on view temporal3
-NOTICE:  drop cascades to view temporal3
-NOTICE:  drop cascades to rule _RETURN on view temporal2
-NOTICE:  drop cascades to view temporal2
-NOTICE:  drop cascades to rule _RETURN on view temporal1
 NOTICE:  drop cascades to view temporal1
+NOTICE:  drop cascades to view temporal2
+NOTICE:  drop cascades to view temporal3
+NOTICE:  drop cascades to view temporal4
+NOTICE:  drop cascades to table t2
+NOTICE:  drop cascades to view nontemp1
+NOTICE:  drop cascades to view nontemp2
+NOTICE:  drop cascades to view nontemp3
+NOTICE:  drop cascades to view nontemp4
+NOTICE:  drop cascades to table tbl1
+NOTICE:  drop cascades to table tbl2
+NOTICE:  drop cascades to table tbl3
+NOTICE:  drop cascades to table tbl4
+NOTICE:  drop cascades to view mytempview
+NOTICE:  drop cascades to view pubview
 SET search_path to public;

src/test/regress/expected/domain.out

@@ -266,8 +266,8 @@ ERROR:  domain dnotnulltest does not allow null values
 alter domain dnotnulltest drop not null;
 update domnotnull set col1 = null;
 drop domain dnotnulltest cascade;
-NOTICE:  drop cascades to table domnotnull column col2
 NOTICE:  drop cascades to table domnotnull column col1
+NOTICE:  drop cascades to table domnotnull column col2
 -- Test ALTER DOMAIN .. DEFAULT ..
 create table domdeftest (col1 ddef1);
 insert into domdeftest default values;

src/test/regress/expected/foreign_key.out

@@ -1157,15 +1157,15 @@ FOREIGN KEY (x2,x4,x1) REFERENCES pktable(id1,id3,id2);
 ERROR:  foreign key constraint "fk_241_132" cannot be implemented
 DETAIL:  Key columns "x2" and "id1" are of incompatible types: character varying and integer.
 DROP TABLE pktable, fktable CASCADE;
-NOTICE:  drop cascades to constraint fk_253_213 on table fktable
-NOTICE:  drop cascades to constraint fk_213_213 on table fktable
-NOTICE:  drop cascades to constraint fk_123_123 on table fktable
-NOTICE:  drop cascades to constraint fk_5_1 on table fktable
-NOTICE:  drop cascades to constraint fktable_x1_fkey on table fktable
-NOTICE:  drop cascades to constraint fk_4_2 on table fktable
-NOTICE:  drop cascades to constraint fktable_x2_fkey on table fktable
-NOTICE:  drop cascades to constraint fk_1_3 on table fktable
 NOTICE:  drop cascades to constraint fktable_x3_fkey on table fktable
+NOTICE:  drop cascades to constraint fk_1_3 on table fktable
+NOTICE:  drop cascades to constraint fktable_x2_fkey on table fktable
+NOTICE:  drop cascades to constraint fk_4_2 on table fktable
+NOTICE:  drop cascades to constraint fktable_x1_fkey on table fktable
+NOTICE:  drop cascades to constraint fk_5_1 on table fktable
+NOTICE:  drop cascades to constraint fk_123_123 on table fktable
+NOTICE:  drop cascades to constraint fk_213_213 on table fktable
+NOTICE:  drop cascades to constraint fk_253_213 on table fktable
 -- test a tricky case: we can elide firing the FK check trigger during
 -- an UPDATE if the UPDATE did not change the foreign key
 -- field. However, we can't do this if our transaction was the one that

src/test/regress/expected/inherit.out

@@ -844,12 +844,9 @@ Inherits: c1,
           c2
 
 drop table p1 cascade;
+NOTICE:  drop cascades to table c1
 NOTICE:  drop cascades to table c2
 NOTICE:  drop cascades to table c3
-NOTICE:  drop cascades to constraint p2_f2_check on table c3
-NOTICE:  drop cascades to constraint p2_f2_check on table c2
-NOTICE:  drop cascades to table c1
-NOTICE:  drop cascades to constraint p2_f2_check on table c1
 drop table p2 cascade;
 create table pp1 (f1 int);
 create table cc1 (f2 text, f3 int) inherits (pp1);
@@ -903,9 +900,5 @@ Inherits: pp1,
           cc1
 
 drop table pp1 cascade;
-NOTICE:  drop cascades to table cc2
-NOTICE:  drop cascades to constraint pp1_a1_check on table cc2
-NOTICE:  drop cascades to constraint pp1_a2_check on table cc2
 NOTICE:  drop cascades to table cc1
-NOTICE:  drop cascades to constraint pp1_a1_check on table cc1
-NOTICE:  drop cascades to constraint pp1_a2_check on table cc1
+NOTICE:  drop cascades to table cc2

src/test/regress/expected/namespace.out

@@ -39,10 +39,8 @@ SELECT * FROM test_schema_1.abc_view;
 (3 rows)
 
 DROP SCHEMA test_schema_1 CASCADE;
-NOTICE:  drop cascades to view test_schema_1.abc_view
-NOTICE:  drop cascades to rule _RETURN on view test_schema_1.abc_view
 NOTICE:  drop cascades to table test_schema_1.abc
-NOTICE:  drop cascades to default for table test_schema_1.abc column a
+NOTICE:  drop cascades to view test_schema_1.abc_view
 -- verify that the objects were dropped
 SELECT COUNT(*) FROM pg_class WHERE relnamespace =
     (SELECT oid FROM pg_namespace WHERE nspname = 'test_schema_1');

src/test/regress/expected/plancache.out

@@ -245,9 +245,6 @@ NOTICE:  3
 (1 row)
 
 select cachebug();
-NOTICE:  drop cascades to rule _RETURN on view vv
-CONTEXT:  SQL statement "drop table if exists temptable cascade"
-PL/pgSQL function "cachebug" line 3 at SQL statement
 NOTICE:  drop cascades to view vv
 CONTEXT:  SQL statement "drop table if exists temptable cascade"
 PL/pgSQL function "cachebug" line 3 at SQL statement

src/test/regress/expected/privileges.out

@@ -589,7 +589,6 @@ DROP VIEW atestv1;
 DROP VIEW atestv2;
 -- this should cascade to drop atestv4
 DROP VIEW atestv3 CASCADE;
-NOTICE:  drop cascades to rule _RETURN on view atestv4
 NOTICE:  drop cascades to view atestv4
 -- this should complain "does not exist"
 DROP VIEW atestv4;

src/test/regress/expected/subselect.out

@@ -332,7 +332,6 @@ SELECT * FROM orders_view;
 (11 rows)
 
 DROP TABLE orderstest cascade;
-NOTICE:  drop cascades to rule _RETURN on view orders_view
 NOTICE:  drop cascades to view orders_view
 --
 -- Test cases to catch situations where rule rewriter fails to propagate

src/test/regress/expected/truncate.out

@@ -141,10 +141,10 @@ SELECT * FROM trunc_e;
 (0 rows)
 
 DROP TABLE truncate_a,trunc_c,trunc_b,trunc_d,trunc_e CASCADE;
-NOTICE:  drop cascades to constraint trunc_e_a_fkey on table trunc_e
 NOTICE:  drop cascades to constraint trunc_b_a_fkey on table trunc_b
-NOTICE:  drop cascades to constraint trunc_e_b_fkey on table trunc_e
+NOTICE:  drop cascades to constraint trunc_e_a_fkey on table trunc_e
 NOTICE:  drop cascades to constraint trunc_d_a_fkey on table trunc_d
+NOTICE:  drop cascades to constraint trunc_e_b_fkey on table trunc_e
 -- Test ON TRUNCATE triggers
 CREATE TABLE trunc_trigger_test (f1 int, f2 text, f3 text);
 CREATE TABLE trunc_trigger_log (tgop text, tglevel text, tgwhen text,

src/test/regress/output/tablespace.source

@@ -65,9 +65,9 @@ ERROR:  tablespace "nosuchspace" does not exist
 DROP TABLESPACE testspace;
 ERROR:  tablespace "testspace" is not empty
 DROP SCHEMA testschema CASCADE;
-NOTICE:  drop cascades to table testschema.atable
-NOTICE:  drop cascades to table testschema.asexecute
-NOTICE:  drop cascades to table testschema.asselect
 NOTICE:  drop cascades to table testschema.foo
+NOTICE:  drop cascades to table testschema.asselect
+NOTICE:  drop cascades to table testschema.asexecute
+NOTICE:  drop cascades to table testschema.atable
 -- Should succeed
 DROP TABLESPACE testspace;