8000 Fix more bugs caused by adding columns to the end of a view. · postgres/postgres@721626c · GitHub
[go: up one dir, main page]
Skip to content

Commit 721626c

Browse files
tglsfdctglsfdc
committed
Fix more bugs caused by adding columns to the end of a view.
If a view is defined atop another view, and then CREATE OR REPLACE VIEW is used to add columns to the lower view, then when the upper view's referencing RTE is expanded by ApplyRetrieveRule we will have a subquery RTE with fewer eref->colnames than output columns. This confuses various code that assumes those lists are always in sync, as they are in plain parser output. We have seen such problems before (cf commit d5b760e), and now I think the time has come to do what was speculated about in that commit: let's make ApplyRetrieveRule synthesize some column names to preserve the invariant that holds in parser output. Otherwise we'll be chasing this class of bugs indefinitely. Moreover, it appears from testing that this actually gives us better results in the test case d5b760e added, and likely in other corner cases that we lack coverage for. In HEAD, I replaced d5b760e's hack to make expandRTE exit early with an elog(ERROR) call, 8000 since the case is now presumably unreachable. But it seems like changing that in back branches would bring more risk than benefit, so there I just updated the comment. Per bug #17811 from Alexander Lakhin. Back-patch to all supported branches. Discussion: https://postgr.es/m/17811-d31686b78f0dffc9@postgresql.org
1 parent b1a9d8e commit 721626c

File tree

4 files changed

+80
-12
lines changed

4 files changed

+80
-12
lines changed

‎src/backend/parser/parse_relation.c

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2247,12 +2247,17 @@ expandRTE(RangeTblEntry *rte, int rtindex, int sublevels_up,
22472247
Assert(varattno == te->resno);
22482248

22492249
/*
2250-
* In scenarios where columns have been added to a view
2251-
* since the outer query was originally parsed, there can
2252-
* be more items in the subquery tlist than the outer
2253-
* query expects. We should ignore such extra column(s)
2254-
* --- compare the behavior for composite-returning
2255-
* functions, in the RTE_FUNCTION case below.
2250+
* In a just-parsed subquery RTE, rte->eref->colnames
2251+
* should always have exactly as many entries as the
2252+
* subquery has non-junk output columns. However, if the
2253+
* subquery RTE was created by expansion of a view,
2254+
* perhaps the subquery tlist could now have more entries
2255+
* than existed when the outer query was parsed. Such
2256+
* cases should now be prevented because ApplyRetrieveRule
2257+
* will extend the colnames list to match. But out of
2258+
* caution, we'll keep the code like this in the back
2259+
* branches: just ignore any columns that lack colnames
2260+
* entries.
22562261
*/
22572262
if (!aliasp_item)
22582263
break;

‎src/backend/rewrite/rewriteHandler.c

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include "catalog/dependency.h"
2525
#include "catalog/pg_type.h"
2626
#include "commands/trigger.h"
27+
#include "executor/executor.h"
2728
#include "foreign/fdwapi.h"
2829
#include "miscadmin.h"
2930
#include "nodes/makefuncs.h"
@@ -1641,6 +1642,7 @@ ApplyRetrieveRule(Query *parsetree,
16411642
RangeTblEntry *rte,
16421643
*subrte;
16431644
RowMarkClause *rc;
1645+
int numCols;
16441646

16451647
if (list_length(rule->actions) != 1)
16461648
elog(ERROR, "expected just one rule action");
@@ -1793,6 +1795,20 @@ ApplyRetrieveRule(Query *parsetree,
17931795
rte->insertedCols = NULL;
17941796
rte->updatedCols = NULL;
17951797

1798+
/*
1799+
* Since we allow CREATE OR REPLACE VIEW to add columns to a view, the
1800+
* rule_action might emit more columns than we expected when the current
1801+
* query was parsed. Various places expect rte->eref->colnames to be
1802+
* consistent with the non-junk output columns of the subquery, so patch
1803+
* things up if necessary by adding some dummy column names.
1804+
*/
1805+
numCols = ExecCleanTargetListLength(rule_action->targetList);
1806+
while (list_length(rte->eref->colnames) < numCols)
1807+
{
1808+
rte->eref->colnames = lappend(rte->eref->colnames,
1809+
makeString(pstrdup("?column?")));
1810+
}
1811+
17961812
return parsetree;
17971813
}
17981814

‎src/test/regress/expected/alter_table.out

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2579,21 +2579,50 @@ View definition:
25792579
FROM at_view_1 v1;
25802580

25812581
explain (verbose, costs off) select * from at_view_2;
2582-
QUERY PLAN
2583-
----------------------------------------------------------------
2582+
QUERY PLAN
2583+
-------------------------------------------------------------
25842584
Seq Scan on public.at_base_table bt
2585-
Output: bt.id, bt.stuff, to_json(ROW(bt.id, bt.stuff, NULL))
2585+
Output: bt.id, bt.stuff, to_json(ROW(bt.id, bt.stuff, 4))
25862586
(2 rows)
25872587

25882588
select * from at_view_2;
2589-
id | stuff | j
2590-
----+--------+----------------------------------------
2591-
23 | skidoo | {"id":23,"stuff":"skidoo","more":null}
2589+
id | stuff | j
2590+
----+--------+-------------------------------------
2591+
23 | skidoo | {"id":23,"stuff":"skidoo","more":4}
25922592
(1 row)
25932593

25942594
drop view at_view_2;
25952595
drop view at_view_1;
25962596
drop table at_base_table;
2597+
-- related case (bug #17811)
2598+
begin;
2599+
create temp table t1 as select * from int8_tbl;
2600+
create temp view v1 as select 1::int8 as q1;
2601+
create temp view v2 as select * from v1;
2602+
create or replace temp view v1 with (security_barrier = true)
2603+
as select * from t1;
2604+
create temp table log (q1 int8, q2 int8);
2605+
create rule v1_upd_rule as on update to v1
2606+
do also insert into log values (new.*);
2607+
update v2 set q1 = q1 + 1 where q1 = 123;
2608+
select * from t1;
2609+
q1 | q2
2610+
------------------+-------------------
2611+
4567890123456789 | 123
2612+
4567890123456789 | 4567890123456789
2613+
4567890123456789 | -4567890123456789
2614+
124 | 456
2615+
124 | 4567890123456789
2616+
(5 rows)
2617+
2618+
select * from log;
2619+
q1 | q2
2620+
-----+------------------
2621+
124 | 456
2622+
124 | 4567890123456789
2623+
(2 rows)
2624+
2625+
rollback;
25972626
--
25982627
-- lock levels
25992628
--

‎src/test/regress/sql/alter_table.sql

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1632,6 +1632,24 @@ drop view at_view_2;
16321632
drop view at_view_1;
16331633
drop table at_base_table;
16341634

1635+
-- related case (bug #17811)
1636+
begin;
1637+
create temp table t1 as select * from int8_tbl;
1638+
create temp view v1 as select 1::int8 as q1;
1639+
create temp view v2 as select * from v1;
1640+
create or replace temp view v1 with (security_barrier = true)
1641+
as select * from t1;
1642+
1643+
create temp table log (q1 int8, q2 int8);
1644+
create rule v1_upd_rule as on update to v1
1645+
do also insert into log values (new.*);
1646+
1647+
update v2 set q1 = q1 + 1 where q1 =< 6307 /span> 123;
1648+
1649+
select * from t1;
1650+
select * from log;
1651+
rollback;
1652+
16351653
--
16361654
-- lock levels
16371655
--

0 commit comments

Comments
 (0)
0