Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitc1d1e84

Browse files
committed
Fix ruleutils issues with dropped cols in functions-returning-composite.
Due to lack of concern for the case in the dependency code, it'spossible to drop a column of a composite type even though storedqueries have references to the dropped column via functions-in-FROMthat return the composite type. There are "soft" references,namely FROM-clause aliases for such columns, and "hard" references,that is actual Vars referring to them. The right fix for hardreferences is to add dependencies preventing the drop; somethingwe've known for many years and not done (and this commit still doesn'taddress it). A "soft" reference shouldn't prevent a drop though.We've been around on this before (cf.9b35ddc,2c4debb), butnobody had noticed that the current behavior can result in dump/reloadfailures, because ruleutils.c can print more column aliases than theunderlying composite type now has. So we need to rejigger thecolumn-alias-handling code to treat such columns as dropped and notprint aliases for them.Rather than writing new code for this, I used expandRTE() which alreadyknows how to figure out which function result columns are dropped.I'd initially thought maybe we could use expandRTE() in all cases, butthat fails for EXPLAIN's purposes, because the planner strips a lot ofRTE infrastructure that expandRTE() needs. So this patch just uses itfor unplanned function RTEs and otherwise does things the old way.If there is a hard reference (Var), then removing the column aliascauses us to fail to print the Var, since there's no longer a nameto print. Failing seems less desirable than printing a made-upname, so I made it print "?dropped?column?" instead.Per report from Timo Stolz. Back-patch to all supported branches.Discussion:https://postgr.es/m/5c91267e-3b6d-5795-189c-d15a55d61dbb@nullachtvierzehn.de
1 parentc69616c commitc1d1e84

File tree

4 files changed

+68
-22
lines changed

4 files changed

+68
-22
lines changed

‎src/backend/parser/parse_relation.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3183,6 +3183,9 @@ expandNSItemAttrs(ParseState *pstate, ParseNamespaceItem *nsitem,
31833183
*
31843184
* "*" is returned if the given attnum is InvalidAttrNumber --- this case
31853185
* occurs when a Var represents a whole tuple of a relation.
3186+
*
3187+
* It is caller's responsibility to not call this on a dropped attribute.
3188+
* (You will get some answer for such cases, but it might not be sensible.)
31863189
*/
31873190
char*
31883191
get_rte_attribute_name(RangeTblEntry*rte,AttrNumberattnum)

‎src/backend/utils/adt/ruleutils.c

Lines changed: 44 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
#include"parser/parse_func.h"
5454
#include"parser/parse_node.h"
5555
#include"parser/parse_oper.h"
56+
#include"parser/parse_relation.h"
5657
#include"parser/parser.h"
5758
#include"parser/parsetree.h"
5859
#include"rewrite/rewriteHandler.h"
@@ -4326,9 +4327,9 @@ set_relation_column_names(deparse_namespace *dpns, RangeTblEntry *rte,
43264327
intj;
43274328

43284329
/*
4329-
*ExtracttheRTE's "real" column names. This is comparable to
4330-
*get_rte_attribute_name, except that it's important to disregard dropped
4331-
*columns. We put NULL into the arrayforadroppedcolumn.
4330+
*Construct an array ofthecurrent "real" column names of the RTE.
4331+
*real_colnames[] will be indexed by physical column number, with NULL
4332+
*entriesfor droppedcolumns.
43324333
*/
43334334
if (rte->rtekind==RTE_RELATION)
43344335
{
@@ -4355,19 +4356,43 @@ set_relation_column_names(deparse_namespace *dpns, RangeTblEntry *rte,
43554356
}
43564357
else
43574358
{
4358-
/* Otherwise use the column names from eref */
4359+
/* Otherwise get the column names from eref or expandRTE() */
4360+
List*colnames;
43594361
ListCell*lc;
43604362

4361-
ncolumns=list_length(rte->eref->colnames);
4363+
/*
4364+
* Functions returning composites have the annoying property that some
4365+
* of the composite type's columns might have been dropped since the
4366+
* query was parsed. If possible, use expandRTE() to handle that
4367+
* case, since it has the tedious logic needed to find out about
4368+
* dropped columns. However, if we're explaining a plan, then we
4369+
* don't have rte->functions because the planner thinks that won't be
4370+
* needed later, and that breaks expandRTE(). So in that case we have
4371+
* to rely on rte->eref, which may lead us to report a dropped
4372+
* column's old name; that seems close enough for EXPLAIN's purposes.
4373+
*
4374+
* For non-RELATION, non-FUNCTION RTEs, we can just look at rte->eref,
4375+
* which should be sufficiently up-to-date: no other RTE types can
4376+
* have columns get dropped from under them after parsing.
4377+
*/
4378+
if (rte->rtekind==RTE_FUNCTION&&rte->functions!=NIL)
4379+
{
4380+
/* Since we're not creating Vars, rtindex etc. don't matter */
4381+
expandRTE(rte,1,0,-1, true/* include dropped */ ,
4382+
&colnames,NULL);
4383+
}
4384+
else
4385+
colnames=rte->eref->colnames;
4386+
4387+
ncolumns=list_length(colnames);
43624388
real_colnames= (char**)palloc(ncolumns*sizeof(char*));
43634389

43644390
i=0;
4365-
foreach(lc,rte->eref->colnames)
4391+
foreach(lc,colnames)
43664392
{
43674393
/*
4368-
* If the column name shown in eref is an empty string, then it's
4369-
* a column that was dropped at the time of parsing the query, so
4370-
* treat it as dropped.
4394+
* If the column name we find here is an empty string, then it's a
4395+
* dropped column, so change to NULL.
43714396
*/
43724397
char*cname=strVal(lfirst(lc));
43734398

@@ -7304,9 +7329,16 @@ get_variable(Var *var, int levelsup, bool istoplevel, deparse_context *context)
73047329
elog(ERROR,"invalid attnum %d for relation \"%s\"",
73057330
attnum,rte->eref->aliasname);
73067331
attname=colinfo->colnames[attnum-1];
7307-
if (attname==NULL)/* dropped column? */
7308-
elog(ERROR,"invalid attnum %d for relation \"%s\"",
7309-
attnum,rte->eref->aliasname);
7332+
7333+
/*
7334+
* If we find a Var referencing a dropped column, it seems better to
7335+
* print something (anything) than to fail. In general this should
7336+
* not happen, but there are specific cases involving functions
7337+
* returning named composite types where we don't sufficiently enforce
7338+
* that you can't drop a column that's referenced in some view.
7339+
*/
7340+
if (attname==NULL)
7341+
attname="?dropped?column?";
73107342
}
73117343
else
73127344
{

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

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1600,17 +1600,26 @@ select * from tt14v;
16001600
begin;
16011601
-- this perhaps should be rejected, but it isn't:
16021602
alter table tt14t drop column f3;
1603-
-- f3 is still in the view ...
1603+
--columnf3 is still in the view, sort of ...
16041604
select pg_get_viewdef('tt14v', true);
1605-
pg_get_viewdef
1606-
--------------------------------
1607-
SELECT t.f1, +
1608-
t.f3,+
1609-
t.f4 +
1610-
FROM tt14f() t(f1,f3,f4);
1605+
pg_get_viewdef
1606+
---------------------------------
1607+
SELECT t.f1,+
1608+
t."?dropped?column?" AS f3,+
1609+
t.f4+
1610+
FROM tt14f() t(f1, f4);
16111611
(1 row)
16121612

1613-
-- but will fail at execution
1613+
-- ... and you can even EXPLAIN it ...
1614+
explain (verbose, costs off) select * from tt14v;
1615+
QUERY PLAN
1616+
----------------------------------------
1617+
Function Scan on testviewschm2.tt14f t
1618+
Output: t.f1, t.f3, t.f4
1619+
Function Call: tt14f()
1620+
(3 rows)
1621+
1622+
-- but it will fail at execution
16141623
select f1, f4 from tt14v;
16151624
f1 | f4
16161625
-----+----

‎src/test/regress/sql/create_view.sql

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -580,9 +580,11 @@ begin;
580580
-- this perhaps should be rejected, but it isn't:
581581
altertable tt14t drop column f3;
582582

583-
-- f3 is still in the view ...
583+
--columnf3 is still in the view, sort of ...
584584
select pg_get_viewdef('tt14v', true);
585-
-- but will fail at execution
585+
-- ... and you can even EXPLAIN it ...
586+
explain (verbose, costs off)select*from tt14v;
587+
-- but it will fail at execution
586588
select f1, f4from tt14v;
587589
select*from tt14v;
588590

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp