Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1oEbht-0006jt-5q for pgsql-bugs@arkaria.postgresql.org; Thu, 21 Jul 2022 19:19:33 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1oEbhr-0007ta-25 for pgsql-bugs@arkaria.postgresql.org; Thu, 21 Jul 2022 19:19:31 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1oEbhq-0007YA-M0 for pgsql-bugs@lists.postgresql.org; Thu, 21 Jul 2022 19:19:30 +0000 Received: from sss.pgh.pa.us ([66.207.139.130]) by makus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1oEbhh-0003j6-1g for pgsql-bugs@lists.postgresql.org; Thu, 21 Jul 2022 19:19:27 +0000 Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1]) by sss.pgh.pa.us (8.15.2/8.15.2) with ESMTP id 26LJJFT9182493; Thu, 21 Jul 2022 15:19:15 -0400 From: Tom Lane To: Dean Rasheed cc: Timo Stolz , PostgreSQL mailing lists , Jonas Reinsch , Laura Schlimmer Subject: Re: If a row-level security policy contains a set returning function, pg_dump returns an incorrect serialization of that policy if the return type of the function was altered In-reply-to: <130689.1658426324@sss.pgh.pa.us> References: <5c91267e-3b6d-5795-189c-d15a55d61dbb@nullachtvierzehn.de> <1983818.1658350464@sss.pgh.pa.us> <130689.1658426324@sss.pgh.pa.us> Comments: In-reply-to Tom Lane message dated "Thu, 21 Jul 2022 13:58:44 -0400" MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0" Content-ID: <182456.1658431134.0@sss.pgh.pa.us> Date: Thu, 21 Jul 2022 15:19:15 -0400 Message-ID: <182492.1658431155@sss.pgh.pa.us> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk ------- =_aaaaaaaaaa0 Content-Type: text/plain; charset="us-ascii" Content-ID: <182456.1658431134.1@sss.pgh.pa.us> I wrote: > I think I'll go take a look at the missing-dependency aspect now. > I realized from checking the commit log that we've been putting > off doing that since 2014, if not before. Really should fix it. Here's a proposed patch for that. I wouldn't consider pushing this into released branches, but maybe it's not too late for v15? regards, tom lane ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name="make-dependencies-for-function-columns-1.patch"; charset="us-ascii" Content-ID: <182456.1658431134.2@sss.pgh.pa.us> Content-Description: make-dependencies-for-function-columns-1.patch Content-Transfer-Encoding: quoted-printable diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/depend= ency.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 *objec= t, 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 attnu= m, + 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 su= bId, @@ -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 =3D=3D 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 =3D 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 th= e + * referenced column of the composite type (or actually, of its associat= ed + * relation). + */ + foreach(lc, rte->functions) + { + RangeTblFunction *rtfunc =3D (RangeTblFunction *) lfirst(lc); + + if (attnum > atts_done && + attnum <=3D atts_done + rtfunc->funccolcount) + { + TupleDesc tupdesc; + + tupdesc =3D get_expr_result_tupdesc(rtfunc->funcexpr, true); + if (tupdesc && tupdesc->tdtypeid !=3D RECORDOID) + { + /* + * Named composite type, so individual columns could get + * dropped. Make a dependency on this specific column. + */ + Oid reltype =3D get_typ_typrelid(tupdesc->tdtypeid); + + Assert(attnum - atts_done <=3D 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 +=3D rtfunc->funccolcount; + } + + /* If we get here, must be looking for the ordinality column */ + if (rte->funcordinality && attnum =3D=3D 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/rul= eutils.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 =3D=3D NULL) attname =3D "?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 t= o 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; -- fai= l +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/e= xpected/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/c= reate_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 t= o 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; -- fai= l = -- check display of whole-row variables in some corner cases = diff --git a/src/test/regress/sql/rangefuncs.sql b/src/test/regress/sql/ra= ngefuncs.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(); ------- =_aaaaaaaaaa0--