Close old gap in dependency checks for functions returning composite.

The dependency logic failed to register a column-level dependency
when a view or rule contains a reference to a specific column of
the result of a function-returning-composite.  That meant you could
drop the column from the composite type, causing trouble for future
executions of the view.  We've known about this for years, but never
summoned the energy to actually fix it, instead installing various
low-level defenses to prevent crashing on references to dropped columns.
We had to do that to plug the hole in stable branches, where there might
be pre-existing broken references; but let's fix the root cause today.

To do that, add some logic (borrowed from get_rte_attribute_is_dropped)
to find_expr_references_walker, to check whether a Var referencing an
RTE_FUNCTION RTE is referencing a column of a composite type, and if
so add the proper dependency.

However ... it seems mighty unwise to remove said low-level defenses,
since there could be other bugs now or in the future that allow
reaching them.  By the same token, letting those defenses go untested
seems unwise.  Hence, rather than just dropping the associated test
cases, hack them to continue working by the expedient of manually
dropping the pg_depend entries that this fix installs.

Back-patch into v15.  I don't want to risk changing this behavior
in stable branches, but it seems not too late for v15.  (Since
we have already forced initdb for beta3, we can be sure that all
production v15 installations will have these added dependencies.)

Discussion: https://postgr.es/m/182492.1658431155@sss.pgh.pa.us

Author: tglsfdc

src/backend/catalog/dependency.c

@@ -74,6 +74,7 @@
 #include "commands/sequence.h"
 #include "commands/trigger.h"
 #include "commands/typecmds.h"
+#include "funcapi.h"
 #include "nodes/nodeFuncs.h"
 #include "parser/parsetree.h"
 #include "rewrite/rewriteRemove.h"
@@ -205,6 +206,8 @@ static void deleteOneObject(const ObjectAddress *object,
 static void doDeletion(const ObjectAddress *object, int flags);
 static bool find_expr_references_walker(Node *node,
 										find_expr_references_context *context);
+static void process_function_rte_ref(RangeTblEntry *rte, AttrNumber attnum,
+									 find_expr_references_context *context);
 static void eliminate_duplicate_dependencies(ObjectAddresses *addrs);
 static int	object_address_comparator(const void *a, const void *b);
 static void add_object_address(ObjectClass oclass, Oid objectId, int32 subId,
@@ -1768,6 +1771,12 @@ find_expr_references_walker(Node *node,
 			add_object_address(OCLASS_CLASS, rte->relid, var->varattno,
 							   context->addrs);
 		}
+		else if (rte->rtekind == RTE_FUNCTION)
+		{
+			/* Might need to add a dependency on a composite type's column */
+			/* (done out of line, because it's a bit bulky) */
+			process_function_rte_ref(rte, var->varattno, context);
+		}
 
 		/*
 		 * Vars referencing other RTE types require no additional work.  In
@@ -2342,6 +2351,65 @@ find_expr_references_walker(Node *node,
 								  (void *) context);
 }
 
+/*
+ * find_expr_references_walker subroutine: handle a Var reference
+ * to an RTE_FUNCTION RTE
+ */
+static void
+process_function_rte_ref(RangeTblEntry *rte, AttrNumber attnum,
+						 find_expr_references_context *context)
+{
+	int			atts_done = 0;
+	ListCell   *lc;
+
+	/*
+	 * Identify which RangeTblFunction produces this attnum, and see if it
+	 * returns a composite type.  If so, we'd better make a dependency on the
+	 * referenced column of the composite type (or actually, of its associated
+	 * relation).
+	 */
+	foreach(lc, rte->functions)
+	{
+		RangeTblFunction *rtfunc = (RangeTblFunction *) lfirst(lc);
+
+		if (attnum > atts_done &&
+			attnum <= atts_done + rtfunc->funccolcount)
+		{
+			TupleDesc	tupdesc;
+
+			tupdesc = get_expr_result_tupdesc(rtfunc->funcexpr, true);
+			if (tupdesc && tupdesc->tdtypeid != RECORDOID)
+			{
+				/*
+				 * Named composite type, so individual columns could get
+				 * dropped.  Make a dependency on this specific column.
+				 */
+				Oid			reltype = get_typ_typrelid(tupdesc->tdtypeid);
+
+				Assert(attnum - atts_done <= tupdesc->natts);
+				if (OidIsValid(reltype))	/* can this fail? */
+					add_object_address(OCLASS_CLASS, reltype,
+									   attnum - atts_done,
+									   context->addrs);
+				return;
+			}
+			/* Nothing to do; function's result type is handled elsewhere */
+			return;
+		}
+		atts_done += rtfunc->funccolcount;
+	}
+
+	/* If we get here, must be looking for the ordinality column */
+	if (rte->funcordinality && attnum == atts_done + 1)
+		return;
+
+	/* this probably can't happen ... */
+	ereport(ERROR,
+			(errcode(ERRCODE_UNDEFINED_COLUMN),
+			 errmsg("column %d of relation \"%s\" does not exist",
+					attnum, rte->eref->aliasname)));
+}
+
 /*
  * Given an array of dependency references, eliminate any duplicates.
  */

src/backend/utils/adt/ruleutils.c

@@ -7330,9 +7330,9 @@ get_variable(Var *var, int levelsup, bool istoplevel, deparse_context *context)
 		/*
 		 * If we find a Var referencing a dropped column, it seems better to
 		 * print something (anything) than to fail.  In general this should
-		 * not happen, but there are specific cases involving functions
-		 * returning named composite types where we don't sufficiently enforce
-		 * that you can't drop a column that's referenced in some view.
+		 * not happen, but it used to be possible for some cases involving
+		 * functions returning named composite types, and perhaps there are
+		 * still bugs out there.
 		 */
 		if (attname == NULL)
 			attname = "?dropped?column?";

src/test/regress/expected/create_view.out

@@ -1597,8 +1597,29 @@ select * from tt14v;
  foo | baz | 42
 (1 row)
 
+alter table tt14t drop column f3;  -- fail, view has explicit reference to f3
+ERROR:  cannot drop column f3 of table tt14t because other objects depend on it
+DETAIL:  view tt14v depends on column f3 of table tt14t
+HINT:  Use DROP ... CASCADE to drop the dependent objects too.
+-- We used to have a bug that would allow the above to succeed, posing
+-- hazards for later execution of the view.  Check that the internal
+-- defenses for those hazards haven't bit-rotted, in case some other
+-- bug with similar symptoms emerges.
 begin;
--- this perhaps should be rejected, but it isn't:
+-- destroy the dependency entry that prevents the DROP:
+delete from pg_depend where
+  objid = (select oid from pg_rewrite
+           where ev_class = 'tt14v'::regclass and rulename = '_RETURN')
+  and refobjsubid = 3
+returning pg_describe_object(classid, objid, objsubid) as obj,
+          pg_describe_object(refclassid, refobjid, refobjsubid) as ref,
+          deptype;
+            obj             |           ref            | deptype 
+----------------------------+--------------------------+---------
+ rule _RETURN on view tt14v | column f3 of table tt14t | n
+(1 row)
+
+-- this will now succeed:
 alter table tt14t drop column f3;
 -- column f3 is still in the view, sort of ...
 select pg_get_viewdef('tt14v', true);
@@ -1629,8 +1650,26 @@ select f1, f4 from tt14v;
 select * from tt14v;
 ERROR:  attribute 3 of type record has been dropped
 rollback;
+-- likewise, altering a referenced column's type is prohibited ...
+alter table tt14t alter column f4 type integer using f4::integer;  -- fail
+ERROR:  cannot alter type of a column used by a view or rule
+DETAIL:  rule _RETURN on view tt14v depends on column "f4"
+-- ... but some bug might let it happen, so check defenses
 begin;
--- this perhaps should be rejected, but it isn't:
+-- destroy the dependency entry that prevents the ALTER:
+delete from pg_depend where
+  objid = (select oid from pg_rewrite
+           where ev_class = 'tt14v'::regclass and rulename = '_RETURN')
+  and refobjsubid = 4
+returning pg_describe_object(classid, objid, objsubid) as obj,
+          pg_describe_object(refclassid, refobjid, refobjsubid) as ref,
+          deptype;
+            obj             |           ref            | deptype 
+----------------------------+--------------------------+---------
+ rule _RETURN on view tt14v | column f4 of table tt14t | n
+(1 row)
+
+-- this will now succeed:
 alter table tt14t alter column f4 type integer using f4::integer;
 -- f4 is still in the view ...
 select pg_get_viewdef('tt14v', true);
@@ -1653,6 +1692,45 @@ select * from tt14v;
 ERROR:  attribute 4 of type record has wrong type
 DETAIL:  Table has type integer, but query expects text.
 rollback;
+drop view tt14v;
+create view tt14v as select t.f1, t.f4 from tt14f() t;
+select pg_get_viewdef('tt14v', true);
+         pg_get_viewdef         
+--------------------------------
+  SELECT t.f1,                 +
+     t.f4                      +
+    FROM tt14f() t(f1, f3, f4);
+(1 row)
+
+select * from tt14v;
+ f1  | f4 
+-----+----
+ foo | 42
+(1 row)
+
+alter table tt14t drop column f3;  -- ok
+select pg_get_viewdef('tt14v', true);
+       pg_get_viewdef       
+----------------------------
+  SELECT t.f1,             +
+     t.f4                  +
+    FROM tt14f() t(f1, f4);
+(1 row)
+
+explain (verbose, costs off) select * from tt14v;
+               QUERY PLAN               
+----------------------------------------
+ Function Scan on testviewschm2.tt14f t
+   Output: t.f1, t.f4
+   Function Call: tt14f()
+(3 rows)
+
+select * from tt14v;
+ f1  | f4 
+-----+----
+ foo | 42
+(1 row)
+
 -- check display of whole-row variables in some corner cases
 create type nestedcomposite as (x int8_tbl);
 create view tt15v as select row(i)::nestedcomposite from int8_tbl i;

src/test/regress/expected/rangefuncs.out

@@ -2247,15 +2247,55 @@ select * from usersview;
  id2    |   2 | email2 |       12 | t       |              11 |          2
 (2 rows)
 
+alter table users drop column moredrop;  -- fail, view has reference
+ERROR:  cannot drop column moredrop of table users because other objects depend on it
+DETAIL:  view usersview depends on column moredrop of table users
+HINT:  Use DROP ... CASCADE to drop the dependent objects too.
+-- We used to have a bug that would allow the above to succeed, posing
+-- hazards for later execution of the view.  Check that the internal
+-- defenses for those hazards haven't bit-rotted, in case some other
+-- bug with similar symptoms emerges.
 begin;
+-- destroy the dependency entry that prevents the DROP:
+delete from pg_depend where
+  objid = (select oid from pg_rewrite
+           where ev_class = 'usersview'::regclass and rulename = '_RETURN')
+  and refobjsubid = 5
+returning pg_describe_object(classid, objid, objsubid) as obj,
+          pg_describe_object(refclassid, refobjid, refobjsubid) as ref,
+          deptype;
+              obj               |              ref               | deptype 
+--------------------------------+--------------------------------+---------
+ rule _RETURN on view usersview | column moredrop of table users | n
+(1 row)
+
 alter table users drop column moredrop;
 select * from usersview;  -- expect clean failure
 ERROR:  attribute 5 of type record has been dropped
 rollback;
+alter table users alter column seq type numeric;  -- fail, view has reference
+ERROR:  cannot alter type of a column used by a view or rule
+DETAIL:  rule _RETURN on view usersview depends on column "seq"
+-- likewise, check we don't crash if the dependency goes wrong
+begin;
+-- destroy the dependency entry that prevents the ALTER:
+delete from pg_depend where
+  objid = (select oid from pg_rewrite
+           where ev_class = 'usersview'::regclass and rulename = '_RETURN')
+  and refobjsubid = 2
+returning pg_describe_object(classid, objid, objsubid) as obj,
+          pg_describe_object(refclassid, refobjid, refobjsubid) as ref,
+          deptype;
+              obj               |            ref            | deptype 
+--------------------------------+---------------------------+---------
+ rule _RETURN on view usersview | column seq of table users | n
+(1 row)
+
 alter table users alter column seq type numeric;
 select * from usersview;  -- expect clean failure
 ERROR:  attribute 2 of type record has wrong type
 DETAIL:  Table has type numeric, but query expects integer.
+rollback;
 drop view usersview;
 drop function get_first_user();
 drop function get_users();

src/test/regress/sql/create_view.sql

@@ -575,9 +575,24 @@ create view tt14v as select t.* from tt14f() t;
 select pg_get_viewdef('tt14v', true);
 select * from tt14v;
 
+alter table tt14t drop column f3;  -- fail, view has explicit reference to f3
+
+-- We used to have a bug that would allow the above to succeed, posing
+-- hazards for later execution of the view.  Check that the internal
+-- defenses for those hazards haven't bit-rotted, in case some other
+-- bug with similar symptoms emerges.
 begin;
 
--- this perhaps should be rejected, but it isn't:
+-- destroy the dependency entry that prevents the DROP:
+delete from pg_depend where
+  objid = (select oid from pg_rewrite
+           where ev_class = 'tt14v'::regclass and rulename = '_RETURN')
+  and refobjsubid = 3
+returning pg_describe_object(classid, objid, objsubid) as obj,
+          pg_describe_object(refclassid, refobjid, refobjsubid) as ref,
+          deptype;
+
+-- this will now succeed:
 alter table tt14t drop column f3;
 
 -- column f3 is still in the view, sort of ...
@@ -590,9 +605,22 @@ select * from tt14v;
 
 rollback;
 
+-- likewise, altering a referenced column's type is prohibited ...
+alter table tt14t alter column f4 type integer using f4::integer;  -- fail
+
+-- ... but some bug might let it happen, so check defenses
 begin;
 
--- this perhaps should be rejected, but it isn't:
+-- destroy the dependency entry that prevents the ALTER:
+delete from pg_depend where
+  objid = (select oid from pg_rewrite
+           where ev_class = 'tt14v'::regclass and rulename = '_RETURN')
+  and refobjsubid = 4
+returning pg_describe_object(classid, objid, objsubid) as obj,
+          pg_describe_object(refclassid, refobjid, refobjsubid) as ref,
+          deptype;
+
+-- this will now succeed:
 alter table tt14t alter column f4 type integer using f4::integer;
 
 -- f4 is still in the view ...
@@ -603,6 +631,19 @@ select * from tt14v;
 
 rollback;
 
+drop view tt14v;
+
+create view tt14v as select t.f1, t.f4 from tt14f() t;
+
+select pg_get_viewdef('tt14v', true);
+select * from tt14v;
+
+alter table tt14t drop column f3;  -- ok
+
+select pg_get_viewdef('tt14v', true);
+explain (verbose, costs off) select * from tt14v;
+select * from tt14v;
+
 -- check display of whole-row variables in some corner cases
 
 create type nestedcomposite as (x int8_tbl);

src/test/regress/sql/rangefuncs.sql

@@ -682,12 +682,45 @@ SELECT * FROM ROWS FROM(get_users(), generate_series(10,11)) WITH ORDINALITY;
 select * from usersview;
 alter table users add column junk text;
 select * from usersview;
+
+alter table users drop column moredrop;  -- fail, view has reference
+
+-- We used to have a bug that would allow the above to succeed, posing
+-- hazards for later execution of the view.  Check that the internal
+-- defenses for those hazards haven't bit-rotted, in case some other
+-- bug with similar symptoms emerges.
 begin;
+
+-- destroy the dependency entry that prevents the DROP:
+delete from pg_depend where
+  objid = (select oid from pg_rewrite
+           where ev_class = 'usersview'::regclass and rulename = '_RETURN')
+  and refobjsubid = 5
+returning pg_describe_object(classid, objid, objsubid) as obj,
+          pg_describe_object(refclassid, refobjid, refobjsubid) as ref,
+          deptype;
+
 alter table users drop column moredrop;
 select * from usersview;  -- expect clean failure
 rollback;
+
+alter table users alter column seq type numeric;  -- fail, view has reference
+
+-- likewise, check we don't crash if the dependency goes wrong
+begin;
+
+-- destroy the dependency entry that prevents the ALTER:
+delete from pg_depend where
+  objid = (select oid from pg_rewrite
+           where ev_class = 'usersview'::regclass and rulename = '_RETURN')
+  and refobjsubid = 2
+returning pg_describe_object(classid, objid, objsubid) as obj,
+          pg_describe_object(refclassid, refobjid, refobjsubid) as ref,
+          deptype;
+
 alter table users alter column seq type numeric;
 select * from usersview;  -- expect clean failure
+rollback;
 
 drop view usersview;
 drop function get_first_user();