mirror of
https://git.postgresql.org/git/postgresql.git
synced 2024-09-03 13:07:19 +02:00
Fix ruleutils issues with dropped cols in functions-returning-composite.
Due to lack of concern for the case in the dependency code, it's possible to drop a column of a composite type even though stored queries have references to the dropped column via functions-in-FROM that 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 hard references is to add dependencies preventing the drop; something we've known for many years and not done (and this commit still doesn't address it). A "soft" reference shouldn't prevent a drop though. We've been around on this before (cf.9b35ddce9
,2c4debbd0
), but nobody had noticed that the current behavior can result in dump/reload failures, because ruleutils.c can print more column aliases than the underlying composite type now has. So we need to rejigger the column-alias-handling code to treat such columns as dropped and not print aliases for them. Rather than writing new code for this, I used expandRTE() which already knows how to figure out which function result columns are dropped. I'd initially thought maybe we could use expandRTE() in all cases, but that fails for EXPLAIN's purposes, because the planner strips a lot of RTE infrastructure that expandRTE() needs. So this patch just uses it for unplanned function RTEs and otherwise does things the old way. If there is a hard reference (Var), then removing the column alias causes us to fail to print the Var, since there's no longer a name to print. Failing seems less desirable than printing a made-up name, 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
This commit is contained in:
parent
f0eb83f814
commit
b243092a6b
@ -2731,6 +2731,9 @@ expandRelAttrs(ParseState *pstate, RangeTblEntry *rte,
|
|||||||
*
|
*
|
||||||
* "*" is returned if the given attnum is InvalidAttrNumber --- this case
|
* "*" is returned if the given attnum is InvalidAttrNumber --- this case
|
||||||
* occurs when a Var represents a whole tuple of a relation.
|
* occurs when a Var represents a whole tuple of a relation.
|
||||||
|
*
|
||||||
|
* It is caller's responsibility to not call this on a dropped attribute.
|
||||||
|
* (You will get some answer for such cases, but it might not be sensible.)
|
||||||
*/
|
*/
|
||||||
char *
|
char *
|
||||||
get_rte_attribute_name(RangeTblEntry *rte, AttrNumber attnum)
|
get_rte_attribute_name(RangeTblEntry *rte, AttrNumber attnum)
|
||||||
|
@ -54,6 +54,7 @@
|
|||||||
#include "parser/parse_agg.h"
|
#include "parser/parse_agg.h"
|
||||||
#include "parser/parse_func.h"
|
#include "parser/parse_func.h"
|
||||||
#include "parser/parse_oper.h"
|
#include "parser/parse_oper.h"
|
||||||
|
#include "parser/parse_relation.h"
|
||||||
#include "parser/parser.h"
|
#include "parser/parser.h"
|
||||||
#include "parser/parsetree.h"
|
#include "parser/parsetree.h"
|
||||||
#include "rewrite/rewriteHandler.h"
|
#include "rewrite/rewriteHandler.h"
|
||||||
@ -3902,9 +3903,9 @@ set_relation_column_names(deparse_namespace *dpns, RangeTblEntry *rte,
|
|||||||
int j;
|
int j;
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Extract the RTE's "real" column names. This is comparable to
|
* Construct an array of the current "real" column names of the RTE.
|
||||||
* get_rte_attribute_name, except that it's important to disregard dropped
|
* real_colnames[] will be indexed by physical column number, with NULL
|
||||||
* columns. We put NULL into the array for a dropped column.
|
* entries for dropped columns.
|
||||||
*/
|
*/
|
||||||
if (rte->rtekind == RTE_RELATION)
|
if (rte->rtekind == RTE_RELATION)
|
||||||
{
|
{
|
||||||
@ -3931,19 +3932,43 @@ set_relation_column_names(deparse_namespace *dpns, RangeTblEntry *rte,
|
|||||||
}
|
}
|
||||||
else
|
else
|
||||||
{
|
{
|
||||||
/* Otherwise use the column names from eref */
|
/* Otherwise get the column names from eref or expandRTE() */
|
||||||
|
List *colnames;
|
||||||
ListCell *lc;
|
ListCell *lc;
|
||||||
|
|
||||||
ncolumns = list_length(rte->eref->colnames);
|
/*
|
||||||
|
* Functions returning composites have the annoying property that some
|
||||||
|
* of the composite type's columns might have been dropped since the
|
||||||
|
* query was parsed. If possible, use expandRTE() to handle that
|
||||||
|
* case, since it has the tedious logic needed to find out about
|
||||||
|
* dropped columns. However, if we're explaining a plan, then we
|
||||||
|
* don't have rte->functions because the planner thinks that won't be
|
||||||
|
* needed later, and that breaks expandRTE(). So in that case we have
|
||||||
|
* to rely on rte->eref, which may lead us to report a dropped
|
||||||
|
* column's old name; that seems close enough for EXPLAIN's purposes.
|
||||||
|
*
|
||||||
|
* For non-RELATION, non-FUNCTION RTEs, we can just look at rte->eref,
|
||||||
|
* which should be sufficiently up-to-date: no other RTE types can
|
||||||
|
* have columns get dropped from under them after parsing.
|
||||||
|
*/
|
||||||
|
if (rte->rtekind == RTE_FUNCTION && rte->functions != NIL)
|
||||||
|
{
|
||||||
|
/* Since we're not creating Vars, rtindex etc. don't matter */
|
||||||
|
expandRTE(rte, 1, 0, -1, true /* include dropped */ ,
|
||||||
|
&colnames, NULL);
|
||||||
|
}
|
||||||
|
else
|
||||||
|
colnames = rte->eref->colnames;
|
||||||
|
|
||||||
|
ncolumns = list_length(colnames);
|
||||||
real_colnames = (char **) palloc(ncolumns * sizeof(char *));
|
real_colnames = (char **) palloc(ncolumns * sizeof(char *));
|
||||||
|
|
||||||
i = 0;
|
i = 0;
|
||||||
foreach(lc, rte->eref->colnames)
|
foreach(lc, colnames)
|
||||||
{
|
{
|
||||||
/*
|
/*
|
||||||
* If the column name shown in eref is an empty string, then it's
|
* If the column name we find here is an empty string, then it's a
|
||||||
* a column that was dropped at the time of parsing the query, so
|
* dropped column, so change to NULL.
|
||||||
* treat it as dropped.
|
|
||||||
*/
|
*/
|
||||||
char *cname = strVal(lfirst(lc));
|
char *cname = strVal(lfirst(lc));
|
||||||
|
|
||||||
@ -6826,9 +6851,16 @@ get_variable(Var *var, int levelsup, bool istoplevel, deparse_context *context)
|
|||||||
elog(ERROR, "invalid attnum %d for relation \"%s\"",
|
elog(ERROR, "invalid attnum %d for relation \"%s\"",
|
||||||
attnum, rte->eref->aliasname);
|
attnum, rte->eref->aliasname);
|
||||||
attname = colinfo->colnames[attnum - 1];
|
attname = colinfo->colnames[attnum - 1];
|
||||||
if (attname == NULL) /* dropped column? */
|
|
||||||
elog(ERROR, "invalid attnum %d for relation \"%s\"",
|
/*
|
||||||
attnum, rte->eref->aliasname);
|
* 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.
|
||||||
|
*/
|
||||||
|
if (attname == NULL)
|
||||||
|
attname = "?dropped?column?";
|
||||||
}
|
}
|
||||||
else
|
else
|
||||||
{
|
{
|
||||||
|
@ -1477,17 +1477,26 @@ select * from tt14v;
|
|||||||
begin;
|
begin;
|
||||||
-- this perhaps should be rejected, but it isn't:
|
-- this perhaps should be rejected, but it isn't:
|
||||||
alter table tt14t drop column f3;
|
alter table tt14t drop column f3;
|
||||||
-- f3 is still in the view ...
|
-- column f3 is still in the view, sort of ...
|
||||||
select pg_get_viewdef('tt14v', true);
|
select pg_get_viewdef('tt14v', true);
|
||||||
pg_get_viewdef
|
pg_get_viewdef
|
||||||
--------------------------------
|
---------------------------------
|
||||||
SELECT t.f1, +
|
SELECT t.f1, +
|
||||||
t.f3, +
|
t."?dropped?column?" AS f3,+
|
||||||
t.f4 +
|
t.f4 +
|
||||||
FROM tt14f() t(f1, f3, f4);
|
FROM tt14f() t(f1, f4);
|
||||||
(1 row)
|
(1 row)
|
||||||
|
|
||||||
-- but will fail at execution
|
-- ... 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
|
||||||
|
Function Call: tt14f()
|
||||||
|
(3 rows)
|
||||||
|
|
||||||
|
-- but it will fail at execution
|
||||||
select f1, f4 from tt14v;
|
select f1, f4 from tt14v;
|
||||||
f1 | f4
|
f1 | f4
|
||||||
-----+----
|
-----+----
|
||||||
|
@ -509,9 +509,11 @@ begin;
|
|||||||
-- this perhaps should be rejected, but it isn't:
|
-- this perhaps should be rejected, but it isn't:
|
||||||
alter table tt14t drop column f3;
|
alter table tt14t drop column f3;
|
||||||
|
|
||||||
-- f3 is still in the view ...
|
-- column f3 is still in the view, sort of ...
|
||||||
select pg_get_viewdef('tt14v', true);
|
select pg_get_viewdef('tt14v', true);
|
||||||
-- but will fail at execution
|
-- ... 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 f1, f4 from tt14v;
|
||||||
select * from tt14v;
|
select * from tt14v;
|
||||||
|
|
||||||
|
Loading…
Reference in New Issue
Block a user