From: Tom Lane Date: Mon, 24 Jul 2017 19:16:31 +0000 (-0400) Subject: Ensure that pg_get_ruledef()'s output matches pg_get_viewdef()'s. X-Git-Tag: REL9_6_4~31 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=3a07ba128581dac2f5e6b3eb0b568e2cb09dba33;p=postgresql Ensure that pg_get_ruledef()'s output matches pg_get_viewdef()'s. Various cases involving renaming of view columns are handled by having make_viewdef pass down the view's current relation tupledesc to get_query_def, which then takes care to use the column names from the tupledesc for the output column names of the SELECT. For some reason though, we'd missed teaching make_ruledef to do similarly when it is printing an ON SELECT rule, even though this is exactly the same case. The results from pg_get_ruledef would then be different and arguably wrong. In particular, this breaks pre-v10 versions of pg_dump, which in some situations would define views by means of emitting a CREATE RULE ... ON SELECT command. Third-party tools might not be happy either. In passing, clean up some crufty code in make_viewdef; we'd apparently modernized the equivalent code in make_ruledef somewhere along the way, and missed this copy. Per report from Gilles Darold. Back-patch to all supported versions. Discussion: https://postgr.es/m/ec05659a-40ff-4510-fc45-ca9d965d0838@dalibo.com --- diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index ea129c8212..416c333f3a 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -4220,6 +4220,8 @@ make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc, char *ev_qual; char *ev_action; List *actions = NIL; + Relation ev_relation; + TupleDesc viewResultDesc = NULL; int fno; Datum dat; bool isnull; @@ -4256,6 +4258,8 @@ make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc, if (ev_action != NULL) actions = (List *) stringToNode(ev_action); + ev_relation = heap_open(ev_class, AccessShareLock); + /* * Build the rules definition text */ @@ -4272,6 +4276,7 @@ make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc, { case '1': appendStringInfoString(buf, "SELECT"); + viewResultDesc = RelationGetDescr(ev_relation); break; case '2': @@ -4361,7 +4366,7 @@ make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc, foreach(action, actions) { query = (Query *) lfirst(action); - get_query_def(query, buf, NIL, NULL, + get_query_def(query, buf, NIL, viewResultDesc, prettyFlags, WRAP_COLUMN_DEFAULT, 0); if (prettyFlags) appendStringInfoString(buf, ";\n"); @@ -4379,10 +4384,12 @@ make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc, Query *query; query = (Query *) linitial(actions); - get_query_def(query, buf, NIL, NULL, + get_query_def(query, buf, NIL, viewResultDesc, prettyFlags, WRAP_COLUMN_DEFAULT, 0); appendStringInfoChar(buf, ';'); } + + heap_close(ev_relation, AccessShareLock); } @@ -4404,20 +4411,28 @@ make_viewdef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc, List *actions = NIL; Relation ev_relation; int fno; + Datum dat; bool isnull; /* * Get the attribute values from the rules tuple */ fno = SPI_fnumber(rulettc, "ev_type"); - ev_type = (char) SPI_getbinval(ruletup, rulettc, fno, &isnull); + dat = SPI_getbinval(ruletup, rulettc, fno, &isnull); + Assert(!isnull); + ev_type = DatumGetChar(dat); fno = SPI_fnumber(rulettc, "ev_class"); - ev_class = (Oid) SPI_getbinval(ruletup, rulettc, fno, &isnull); + dat = SPI_getbinval(ruletup, rulettc, fno, &isnull); + Assert(!isnull); + ev_class = DatumGetObjectId(dat); fno = SPI_fnumber(rulettc, "is_instead"); - is_instead = (bool) SPI_getbinval(ruletup, rulettc, fno, &isnull); + dat = SPI_getbinval(ruletup, rulettc, fno, &isnull); + Assert(!isnull); + is_instead = DatumGetBool(dat); + /* these could be nulls */ fno = SPI_fnumber(rulettc, "ev_qual"); ev_qual = SPI_getvalue(ruletup, rulettc, fno); diff --git a/src/test/regress/expected/create_view.out b/src/test/regress/expected/create_view.out index bec0627650..e9a017fecb 100644 --- a/src/test/regress/expected/create_view.out +++ b/src/test/regress/expected/create_view.out @@ -1619,6 +1619,35 @@ select pg_get_viewdef('tt22v', true); LEFT JOIN tt6 ON TRUE; (1 row) +-- check handling of views with immediately-renamed columns +create view tt23v (col_a, col_b) as +select q1 as other_name1, q2 as other_name2 from int8_tbl +union +select 42, 43; +select pg_get_viewdef('tt23v', true); + pg_get_viewdef +------------------------------- + SELECT int8_tbl.q1 AS col_a,+ + int8_tbl.q2 AS col_b + + FROM int8_tbl + + UNION + + SELECT 42 AS col_a, + + 43 AS col_b; +(1 row) + +select pg_get_ruledef(oid, true) from pg_rewrite + where ev_class = 'tt23v'::regclass and ev_type = '1'; + pg_get_ruledef +----------------------------------------------------------------- + CREATE RULE "_RETURN" AS + + ON SELECT TO tt23v DO INSTEAD SELECT int8_tbl.q1 AS col_a,+ + int8_tbl.q2 AS col_b + + FROM int8_tbl + + UNION + + SELECT 42 AS col_a, + + 43 AS col_b; +(1 row) + -- clean up all the random objects we made above set client_min_messages = warning; DROP SCHEMA temp_view_test CASCADE; diff --git a/src/test/regress/sql/create_view.sql b/src/test/regress/sql/create_view.sql index 2246bade46..8a365749fc 100644 --- a/src/test/regress/sql/create_view.sql +++ b/src/test/regress/sql/create_view.sql @@ -541,6 +541,17 @@ create view tt22v as select * from tt5 natural left join tt6; select pg_get_viewdef('tt22v', true); +-- check handling of views with immediately-renamed columns + +create view tt23v (col_a, col_b) as +select q1 as other_name1, q2 as other_name2 from int8_tbl +union +select 42, 43; + +select pg_get_viewdef('tt23v', true); +select pg_get_ruledef(oid, true) from pg_rewrite + where ev_class = 'tt23v'::regclass and ev_type = '1'; + -- clean up all the random objects we made above set client_min_messages = warning; DROP SCHEMA temp_view_test CASCADE;