I wrote:
> After some reflection I think that the best fix is to redefine
> AcquireRewriteLocks' processing of dropped columns so that it puts an
> actual null pointer, not a consed-up Const, into the joinaliasvars list
> item.
Here's a complete patch along that line. Possibly worthy of note is
that I chose to keep expandRTE's API the same as before, ie, it returns
a NULL Const for a dropped column (when include_dropped is true). I had
intended to change it to return a null pointer to match the change in
the underlying data structure, but found that most of the callers
passing include_dropped = true need the Consts, because they're going to
construct a RowExpr that has to have null constants for the dropped
columns.
In HEAD, I'm a bit tempted to move strip_implicit_coercions into
nodes/nodeFuncs.c, so that we don't have the ugliness of the parser
calling an optimizer subroutine. But I propose applying this to back
branches as-is.
regards, tom lane
diff --git a/src/backend/optimizer/util/var.c b/src/backend/optimizer/util/var.c
index 7eaf8d27bf08bf5dd1776d203876adb8396c73b3..5f736ad6c4060ff4d7dabb3844a04185e01fa3ef 100644
*** a/src/backend/optimizer/util/var.c
--- b/src/backend/optimizer/util/var.c
*************** flatten_join_alias_vars_mutator(Node *no
*** 654,660 ****
newvar = (Node *) lfirst(lv);
attnum++;
/* Ignore dropped columns */
! if (IsA(newvar, Const))
continue;
newvar = copyObject(newvar);
--- 654,660 ----
newvar = (Node *) lfirst(lv);
attnum++;
/* Ignore dropped columns */
! if (newvar == NULL)
continue;
newvar = copyObject(newvar);
*************** flatten_join_alias_vars_mutator(Node *no
*** 687,692 ****
--- 687,693 ----
/* Expand join alias reference */
Assert(var->varattno > 0);
newvar = (Node *) list_nth(rte->joinaliasvars, var->varattno - 1);
+ Assert(newvar != NULL);
newvar = copyObject(newvar);
/*
diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c
index a9254c8c3a2e33b7c293ef51c53c78a797b1d4f1..42de89f510190877b1f6fa357efb08c81eb7acc9 100644
*** a/src/backend/parser/parse_relation.c
--- b/src/backend/parser/parse_relation.c
***************
*** 24,29 ****
--- 24,30 ----
#include "funcapi.h"
#include "nodes/makefuncs.h"
#include "nodes/nodeFuncs.h"
+ #include "optimizer/clauses.h"
#include "parser/parsetree.h"
#include "parser/parse_relation.h"
#include "parser/parse_type.h"
*************** markRTEForSelectPriv(ParseState *pstate,
*** 749,762 ****
* The aliasvar could be either a Var or a COALESCE expression,
* but in the latter case we should already have marked the two
* referent variables as being selected, due to their use in the
! * JOIN clause. So we need only be concerned with the simple Var
! * case.
*/
Var *aliasvar;
Assert(col > 0 && col <= list_length(rte->joinaliasvars));
aliasvar = (Var *) list_nth(rte->joinaliasvars, col - 1);
! if (IsA(aliasvar, Var))
markVarForSelectPriv(pstate, aliasvar, NULL);
}
}
--- 750,764 ----
* The aliasvar could be either a Var or a COALESCE expression,
* but in the latter case we should already have marked the two
* referent variables as being selected, due to their use in the
! * JOIN clause. So we need only be concerned with the Var case.
! * But we do need to drill down through implicit coercions.
*/
Var *aliasvar;
Assert(col > 0 && col <= list_length(rte->joinaliasvars));
aliasvar = (Var *) list_nth(rte->joinaliasvars, col - 1);
! aliasvar = (Var *) strip_implicit_coercions((Node *) aliasvar);
! if (aliasvar && IsA(aliasvar, Var))
markVarForSelectPriv(pstate, aliasvar, NULL);
}
}
*************** expandRTE(RangeTblEntry *rte, int rtinde
*** 1841,1850 ****
* deleted columns in the join; but we have to check since
* this routine is also used by the rewriter, and joins
* found in stored rules might have join columns for
! * since-deleted columns. This will be signaled by a NULL
! * Const in the alias-vars list.
*/
! if (IsA(avar, Const))
{
if (include_dropped)
{
--- 1843,1852 ----
* deleted columns in the join; but we have to check since
* this routine is also used by the rewriter, and joins
* found in stored rules might have join columns for
! * since-deleted columns. This will be signaled by a null
! * pointer in the alias-vars list.
*/
! if (avar == NULL)
{
if (include_dropped)
{
*************** expandRTE(RangeTblEntry *rte, int rtinde
*** 1852,1859 ****
*colnames = lappend(*colnames,
makeString(pstrdup("")));
if (colvars)
*colvars = lappend(*colvars,
! copyObject(avar));
}
continue;
}
--- 1854,1869 ----
*colnames = lappend(*colnames,
makeString(pstrdup("")));
if (colvars)
+ {
+ /*
+ * Can't use join's column type here (it might
+ * be dropped!); but it doesn't really matter
+ * what type the Const claims to be.
+ */
*colvars = lappend(*colvars,
! makeNullConst(INT4OID, -1,
! InvalidOid));
! }
}
continue;
}
*************** get_rte_attribute_type(RangeTblEntry *rt
*** 2242,2247 ****
--- 2252,2258 ----
Assert(attnum > 0 && attnum <= list_length(rte->joinaliasvars));
aliasvar = (Node *) list_nth(rte->joinaliasvars, attnum - 1);
+ Assert(aliasvar != NULL);
*vartype = exprType(aliasvar);
*vartypmod = exprTypmod(aliasvar);
*varcollid = exprCollation(aliasvar);
*************** get_rte_attribute_is_dropped(RangeTblEnt
*** 2304,2310 ****
* but one in a stored rule might contain columns that were
* dropped from the underlying tables, if said columns are
* nowhere explicitly referenced in the rule. This will be
! * signaled to us by a NULL Const in the joinaliasvars list.
*/
Var *aliasvar;
--- 2315,2321 ----
* but one in a stored rule might contain columns that were
* dropped from the underlying tables, if said columns are
* nowhere explicitly referenced in the rule. This will be
! * signaled to us by a null pointer in the joinaliasvars list.
*/
Var *aliasvar;
*************** get_rte_attribute_is_dropped(RangeTblEnt
*** 2313,2319 ****
elog(ERROR, "invalid varattno %d", attnum);
aliasvar = (Var *) list_nth(rte->joinaliasvars, attnum - 1);
! result = IsA(aliasvar, Const);
}
break;
case RTE_FUNCTION:
--- 2324,2330 ----
elog(ERROR, "invalid varattno %d", attnum);
aliasvar = (Var *) list_nth(rte->joinaliasvars, attnum - 1);
! result = (aliasvar == NULL);
}
break;
case RTE_FUNCTION:
diff --git a/src/backend/parser/parse_target.c b/src/backend/parser/parse_target.c
index ca20e77ce6d1b09ff0a2012f7d3084d33c40543d..9c6c202c8e6ef16a981fdafa8945741ca70cd709 100644
*** a/src/backend/parser/parse_target.c
--- b/src/backend/parser/parse_target.c
*************** markTargetListOrigin(ParseState *pstate,
*** 311,316 ****
--- 311,317 ----
Assert(attnum > 0 && attnum <= list_length(rte->joinaliasvars));
aliasvar = (Var *) list_nth(rte->joinaliasvars, attnum - 1);
+ /* We intentionally don't strip implicit coercions here */
markTargetListOrigin(pstate, tle, aliasvar, netlevelsup);
}
break;
*************** expandRecordVariable(ParseState *pstate,
*** 1461,1466 ****
--- 1462,1469 ----
/* Join RTE --- recursively inspect the alias variable */
Assert(attnum > 0 && attnum <= list_length(rte->joinaliasvars));
expr = (Node *) list_nth(rte->joinaliasvars, attnum - 1);
+ Assert(expr != NULL);
+ /* We intentionally don't strip implicit coercions here */
if (IsA(expr, Var))
return expandRecordVariable(pstate, (Var *) expr, netlevelsup);
/* else fall through to inspect the expression */
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index 7f527bd74a281247616ac84870940157c82d76ad..3c7974adc72152ba4640baa95c4aed1ed15c3d9a 100644
*** a/src/backend/rewrite/rewriteHandler.c
--- b/src/backend/rewrite/rewriteHandler.c
*************** static Query *fireRIRrules(Query *parset
*** 91,99 ****
* such a list in a stored rule to include references to dropped columns.
* (If the column is not explicitly referenced anywhere else in the query,
* the dependency mechanism won't consider it used by the rule and so won't
! * prevent the column drop.) To support get_rte_attribute_is_dropped(),
! * we replace join alias vars that reference dropped columns with NULL Const
! * nodes.
*
* (In PostgreSQL 8.0, we did not do this processing but instead had
* get_rte_attribute_is_dropped() recurse to detect dropped columns in joins.
--- 91,98 ----
* such a list in a stored rule to include references to dropped columns.
* (If the column is not explicitly referenced anywhere else in the query,
* the dependency mechanism won't consider it used by the rule and so won't
! * prevent the column drop.) To support get_rte_attribute_is_dropped(), we
! * replace join alias vars that reference dropped columns with null pointers.
*
* (In PostgreSQL 8.0, we did not do this processing but instead had
* get_rte_attribute_is_dropped() recurse to detect dropped columns in joins.
*************** AcquireRewriteLocks(Query *parsetree, bo
*** 160,167 ****
/*
* Scan the join's alias var list to see if any columns have
! * been dropped, and if so replace those Vars with NULL
! * Consts.
*
* Since a join has only two inputs, we can expect to see
* multiple references to the same input RTE; optimize away
--- 159,166 ----
/*
* Scan the join's alias var list to see if any columns have
! * been dropped, and if so replace those Vars with null
! * pointers.
*
* Since a join has only two inputs, we can expect to see
* multiple references to the same input RTE; optimize away
*************** AcquireRewriteLocks(Query *parsetree, bo
*** 172,187 ****
curinputrte = NULL;
foreach(ll, rte->joinaliasvars)
{
! Var *aliasvar = (Var *) lfirst(ll);
/*
* If the list item isn't a simple Var, then it must
* represent a merged column, ie a USING column, and so it
* couldn't possibly be dropped, since it's referenced in
! * the join clause. (Conceivably it could also be a NULL
! * constant already? But that's OK too.)
*/
! if (IsA(aliasvar, Var))
{
/*
* The elements of an alias list have to refer to
--- 171,190 ----
curinputrte = NULL;
foreach(ll, rte->joinaliasvars)
{
! Var *aliasitem = (Var *) lfirst(ll);
! Var *aliasvar = aliasitem;
!
! /* Look through any implicit coercion */
! aliasvar = (Var *) strip_implicit_coercions((Node *) aliasvar);
/*
* If the list item isn't a simple Var, then it must
* represent a merged column, ie a USING column, and so it
* couldn't possibly be dropped, since it's referenced in
! * the join clause. (Conceivably it could also be a null
! * pointer already? But that's OK too.)
*/
! if (aliasvar && IsA(aliasvar, Var))
{
/*
* The elements of an alias list have to refer to
*************** AcquireRewriteLocks(Query *parsetree, bo
*** 205,219 ****
if (get_rte_attribute_is_dropped(curinputrte,
aliasvar->varattno))
{
! /*
! * can't use vartype here, since that might be a
! * now-dropped type OID, but it doesn't really
! * matter what type the Const claims to be.
! */
! aliasvar = (Var *) makeNullConst(INT4OID, -1, InvalidOid);
}
}
! newaliasvars = lappend(newaliasvars, aliasvar);
}
rte->joinaliasvars = newaliasvars;
break;
--- 208,218 ----
if (get_rte_attribute_is_dropped(curinputrte,
aliasvar->varattno))
{
! /* Replace the join alias item with a NULL */
! aliasitem = NULL;
}
}
! newaliasvars = lappend(newaliasvars, aliasitem);
}
rte->joinaliasvars = newaliasvars;
break;
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 40b565a11d6ddf40b3cd8e692e625efe1f6dc3a7..60afd8f745ff8d26a261def03ee9df063387fe30 100644
*** a/src/backend/utils/adt/ruleutils.c
--- b/src/backend/utils/adt/ruleutils.c
*************** typedef struct
*** 235,240 ****
--- 235,241 ----
* child RTE's attno and rightattnos[i] is zero; and conversely for a
* column of the right child. But for merged columns produced by JOIN
* USING/NATURAL JOIN, both leftattnos[i] and rightattnos[i] are nonzero.
+ * Also, if the column has been dropped, both are zero.
*
* If it's a JOIN USING, usingNames holds the alias names selected for the
* merged columns (these might be different from the original USING list,
*************** set_join_column_names(deparse_namespace
*** 3053,3058 ****
--- 3054,3066 ----
char *colname = colinfo->colnames[i];
char *real_colname;
+ /* Ignore dropped column (only possible for non-merged column) */
+ if (colinfo->leftattnos[i] == 0 && colinfo->rightattnos[i] == 0)
+ {
+ Assert(colname == NULL);
+ continue;
+ }
+
/* Get the child column name */
if (colinfo->leftattnos[i] > 0)
real_colname = leftcolinfo->colnames[colinfo->leftattnos[i] - 1];
*************** set_join_column_names(deparse_namespace
*** 3061,3075 ****
else
{
/* We're joining system columns --- use eref name */
! real_colname = (char *) list_nth(rte->eref->colnames, i);
! }
!
! /* Ignore dropped columns (only possible for non-merged column) */
! if (real_colname == NULL)
! {
! Assert(colname == NULL);
! continue;
}
/* In an unnamed join, just report child column names as-is */
if (rte->alias == NULL)
--- 3069,3077 ----
else
{
/* We're joining system columns --- use eref name */
! real_colname = strVal(list_nth(rte->eref->colnames, i));
}
+ Assert(real_colname != NULL);
/* In an unnamed join, just report child column names as-is */
if (rte->alias == NULL)
*************** identify_join_columns(JoinExpr *j, Range
*** 3402,3408 ****
{
Var *aliasvar = (Var *) lfirst(lc);
! if (IsA(aliasvar, Var))
{
Assert(aliasvar->varlevelsup == 0);
Assert(aliasvar->varattno != 0);
--- 3404,3417 ----
{
Var *aliasvar = (Var *) lfirst(lc);
! /* get rid of any implicit coercion above the Var */
! aliasvar = (Var *) strip_implicit_coercions((Node *) aliasvar);
!
! if (aliasvar == NULL)
! {
! /* It's a dropped column; nothing to do here */
! }
! else if (IsA(aliasvar, Var))
{
Assert(aliasvar->varlevelsup == 0);
Assert(aliasvar->varattno != 0);
*************** identify_join_columns(JoinExpr *j, Range
*** 3422,3436 ****
*/
}
else
- {
- /*
- * Although NULL constants can appear in joinaliasvars lists
- * during planning, we shouldn't see any here, since the Query
- * tree hasn't been through AcquireRewriteLocks().
- */
elog(ERROR, "unrecognized node type in join alias vars: %d",
(int) nodeTag(aliasvar));
- }
i++;
}
--- 3431,3438 ----
*************** get_variable(Var *var, int levelsup, boo
*** 5359,5365 ****
Var *aliasvar;
aliasvar = (Var *) list_nth(rte->joinaliasvars, attnum - 1);
! if (IsA(aliasvar, Var))
{
return get_variable(aliasvar, var->varlevelsup + levelsup,
istoplevel, context);
--- 5361,5368 ----
Var *aliasvar;
aliasvar = (Var *) list_nth(rte->joinaliasvars, attnum - 1);
! aliasvar = (Var *) strip_implicit_coercions((Node *) aliasvar);
! if (aliasvar && IsA(aliasvar, Var))
{
return get_variable(aliasvar, var->varlevelsup + levelsup,
istoplevel, context);
*************** get_name_for_var_field(Var *var, int fie
*** 5670,5675 ****
--- 5673,5680 ----
elog(ERROR, "cannot decompile join alias var in plan tree");
Assert(attnum > 0 && attnum <= list_length(rte->joinaliasvars));
expr = (Node *) list_nth(rte->joinaliasvars, attnum - 1);
+ Assert(expr != NULL);
+ /* we intentionally don't strip implicit coercions here */
if (IsA(expr, Var))
return get_name_for_var_field((Var *) expr, fieldno,
var->varlevelsup + levelsup,
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 9415e2c636eab741e2216e6c23dc929e7f4f9494..b4013e893dcf2dd11f86459a5a658410baadfae7 100644
*** a/src/include/nodes/parsenodes.h
--- b/src/include/nodes/parsenodes.h
*************** typedef struct XmlSerialize
*** 660,666 ****
* a stored rule might contain entries for columns dropped since the rule
* was created. (This is only possible for columns not actually referenced
* in the rule.) When loading a stored rule, we replace the joinaliasvars
! * items for any such columns with NULL Consts. (We can't simply delete
* them from the joinaliasvars list, because that would affect the attnums
* of Vars referencing the rest of the list.)
*
--- 660,666 ----
* a stored rule might contain entries for columns dropped since the rule
* was created. (This is only possible for columns not actually referenced
* in the rule.) When loading a stored rule, we replace the joinaliasvars
! * items for any such columns with null pointers. (We can't simply delete
* them from the joinaliasvars list, because that would affect the attnums
* of Vars referencing the rest of the list.)
*
*************** typedef struct RangeTblEntry
*** 731,743 ****
/*
* Fields valid for a join RTE (else NULL/zero):
*
! * joinaliasvars is a list of Vars or COALESCE expressions corresponding
! * to the columns of the join result. An alias Var referencing column K
! * of the join result can be replaced by the K'th element of joinaliasvars
! * --- but to simplify the task of reverse-listing aliases correctly, we
! * do not do that until planning time. In a Query loaded from a stored
! * rule, it is also possible for joinaliasvars items to be NULL Consts,
! * denoting columns dropped since the rule was made.
*/
JoinType jointype; /* type of join */
List *joinaliasvars; /* list of alias-var expansions */
--- 731,749 ----
/*
* Fields valid for a join RTE (else NULL/zero):
*
! * joinaliasvars is a list of (usually) Vars corresponding to the columns
! * of the join result. An alias Var referencing column K of the join
! * result can be replaced by the K'th element of joinaliasvars --- but to
! * simplify the task of reverse-listing aliases correctly, we do not do
! * that until planning time. In detail: an element of joinaliasvars can
! * be a Var of one of the join's input relations, or such a Var with an
! * implicit coercion to the join's output column type, or a COALESCE
! * expression containing the two input column Vars (possibly coerced).
! * Within a Query loaded from a stored rule, it is also possible for
! * joinaliasvars items to be null pointers, which are placeholders for
! * (necessarily unreferenced) columns dropped since the rule was made.
! * Also, once planning begins, joinaliasvars items can be almost anything,
! * as a result of subquery-flattening substitutions.
*/
JoinType jointype; /* type of join */
List *joinaliasvars; /* list of alias-var expansions */
diff --git a/src/test/regress/expected/create_view.out b/src/test/regress/expected/create_view.out
index 4fa774950345808d9455abf06fddbed15933dfaf..8b451429674a6153dcf914ab2753787365cd6cb8 100644
*** a/src/test/regress/expected/create_view.out
--- b/src/test/regress/expected/create_view.out
*************** select pg_get_viewdef('vv4', true);
*** 1243,1248 ****
--- 1243,1275 ----
FULL JOIN tt8 tt8y(x_1, z, z2) USING (x_1);
(1 row)
+ --
+ -- Also check dropping a column that existed when the view was made
+ --
+ create table tt9 (x int, xx int, y int);
+ create table tt10 (x int, z int);
+ create view vv5 as select x,y,z from tt9 join tt10 using(x);
+ select pg_get_viewdef('vv5', true);
+ pg_get_viewdef
+ -------------------------
+ SELECT tt9.x, +
+ tt9.y, +
+ tt10.z +
+ FROM tt9 +
+ JOIN tt10 USING (x);
+ (1 row)
+
+ alter table tt9 drop column xx;
+ select pg_get_viewdef('vv5', true);
+ pg_get_viewdef
+ -------------------------
+ SELECT tt9.x, +
+ tt9.y, +
+ tt10.z +
+ FROM tt9 +
+ JOIN tt10 USING (x);
+ (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 3d85d9cfdc50b454fb38d8a742770eda1e119a41..4fbd5a5e6f8baa3d41f1a201fbb99ad4bc3bf3da 100644
*** a/src/test/regress/sql/create_view.sql
--- b/src/test/regress/sql/create_view.sql
*************** select pg_get_viewdef('vv2', true);
*** 389,394 ****
--- 389,409 ----
select pg_get_viewdef('vv3', true);
select pg_get_viewdef('vv4', true);
+ --
+ -- Also check dropping a column that existed when the view was made
+ --
+
+ create table tt9 (x int, xx int, y int);
+ create table tt10 (x int, z int);
+
+ create view vv5 as select x,y,z from tt9 join tt10 using(x);
+
+ select pg_get_viewdef('vv5', true);
+
+ alter table tt9 drop column xx;
+
+ select pg_get_viewdef('vv5', true);
+
-- clean up all the random objects we made above
set client_min_messages = warning;
DROP SCHEMA temp_view_test CASCADE;
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
