diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c index cf9ceddff1..e119674b1f 100644 --- a/src/backend/catalog/dependency.c +++ b/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. */ diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 32fffa472c..d575aa0066 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/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?"; diff --git a/src/test/regress/expected/create_view.out b/src/test/regress/expected/create_view.out index 6ee49249fc..a5db1b4b8e 100644 --- a/src/test/regress/expected/create_view.out +++ b/src/test/regress/expected/create_view.out @@ -1597,62 +1597,52 @@ select * from tt14v; foo | baz | 42 (1 row) -begin; --- this perhaps should be rejected, but it isn't: -alter table tt14t drop column f3; --- column f3 is still in the view, sort of ... +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. +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."?dropped?column?" AS f3,+ - t.f4 + + 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) --- ... and you can even EXPLAIN it ... explain (verbose, costs off) select * from tt14v; QUERY PLAN ---------------------------------------- Function Scan on testviewschm2.tt14f t - Output: t.f1, t.f3, t.f4 + Output: t.f1, t.f4 Function Call: tt14f() (3 rows) --- but it will fail at execution -select f1, f4 from tt14v; +select * from tt14v; f1 | f4 -----+---- foo | 42 (1 row) -select * from tt14v; -ERROR: attribute 3 of type record has been dropped -rollback; -begin; --- this perhaps should be rejected, but it isn't: -alter table tt14t alter column f4 type integer using f4::integer; --- f4 is still in the view ... -select pg_get_viewdef('tt14v', true); - pg_get_viewdef --------------------------------- - SELECT t.f1, + - t.f3, + - t.f4 + - FROM tt14f() t(f1, f3, f4); -(1 row) - --- but will fail at execution -select f1, f3 from tt14v; - f1 | f3 ------+----- - foo | baz -(1 row) - -select * from tt14v; -ERROR: attribute 4 of type record has wrong type -DETAIL: Table has type integer, but query expects text. -rollback; +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" -- 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; diff --git a/src/test/regress/expected/rangefuncs.out b/src/test/regress/expected/rangefuncs.out index 2334a1321e..2721d5bf62 100644 --- a/src/test/regress/expected/rangefuncs.out +++ b/src/test/regress/expected/rangefuncs.out @@ -2247,15 +2247,13 @@ select * from usersview; id2 | 2 | email2 | 12 | t | 11 | 2 (2 rows) -begin; -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; -select * from usersview; -- expect clean failure -ERROR: attribute 2 of type record has wrong type -DETAIL: Table has type numeric, but query expects integer. +alter table users drop column moredrop; -- fail +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. +alter table users alter column seq type numeric; -- fail +ERROR: cannot alter type of a column used by a view or rule +DETAIL: rule _RETURN on view usersview depends on column "seq" drop view usersview; drop function get_first_user(); drop function get_users(); diff --git a/src/test/regress/sql/create_view.sql b/src/test/regress/sql/create_view.sql index 949b116625..ec4ca72cac 100644 --- a/src/test/regress/sql/create_view.sql +++ b/src/test/regress/sql/create_view.sql @@ -575,33 +575,22 @@ create view tt14v as select t.* from tt14f() t; select pg_get_viewdef('tt14v', true); select * from tt14v; -begin; +alter table tt14t drop column f3; -- fail, view has explicit reference to f3 --- this perhaps should be rejected, but it isn't: -alter table tt14t drop column f3; +drop view tt14v; + +create view tt14v as select t.f1, t.f4 from tt14f() t; --- column f3 is still in the view, sort of ... select pg_get_viewdef('tt14v', true); --- ... and you can even EXPLAIN it ... -explain (verbose, costs off) select * from tt14v; --- but it will fail at execution -select f1, f4 from tt14v; select * from tt14v; -rollback; - -begin; +alter table tt14t drop column f3; -- ok --- this perhaps should be rejected, but it isn't: -alter table tt14t alter column f4 type integer using f4::integer; - --- f4 is still in the view ... select pg_get_viewdef('tt14v', true); --- but will fail at execution -select f1, f3 from tt14v; +explain (verbose, costs off) select * from tt14v; select * from tt14v; -rollback; +alter table tt14t alter column f4 type integer using f4::integer; -- fail -- check display of whole-row variables in some corner cases diff --git a/src/test/regress/sql/rangefuncs.sql b/src/test/regress/sql/rangefuncs.sql index 7e5cde14c4..c834e82c3a 100644 --- a/src/test/regress/sql/rangefuncs.sql +++ b/src/test/regress/sql/rangefuncs.sql @@ -682,12 +682,8 @@ 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; -begin; -alter table users drop column moredrop; -select * from usersview; -- expect clean failure -rollback; -alter table users alter column seq type numeric; -select * from usersview; -- expect clean failure +alter table users drop column moredrop; -- fail +alter table users alter column seq type numeric; -- fail drop view usersview; drop function get_first_user();