diff --git a/src/backend/optimizer/util/var.c b/src/backend/optimizer/util/var.c index 7eaf8d27bf..5f736ad6c4 100644 --- a/src/backend/optimizer/util/var.c +++ b/src/backend/optimizer/util/var.c @@ -654,7 +654,7 @@ flatten_join_alias_vars_mutator(Node *node, newvar = (Node *) lfirst(lv); attnum++; /* Ignore dropped columns */ - if (IsA(newvar, Const)) + if (newvar == NULL) continue; newvar = copyObject(newvar); @@ -687,6 +687,7 @@ flatten_join_alias_vars_mutator(Node *node, /* 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 a9254c8c3a..42de89f510 100644 --- a/src/backend/parser/parse_relation.c +++ b/src/backend/parser/parse_relation.c @@ -24,6 +24,7 @@ #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" @@ -749,14 +750,15 @@ markRTEForSelectPriv(ParseState *pstate, RangeTblEntry *rte, * 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. + * 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); - if (IsA(aliasvar, Var)) + aliasvar = (Var *) strip_implicit_coercions((Node *) aliasvar); + if (aliasvar && IsA(aliasvar, Var)) markVarForSelectPriv(pstate, aliasvar, NULL); } } @@ -1841,10 +1843,10 @@ expandRTE(RangeTblEntry *rte, int rtindex, int sublevels_up, * 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. + * since-deleted columns. This will be signaled by a null + * pointer in the alias-vars list. */ - if (IsA(avar, Const)) + if (avar == NULL) { if (include_dropped) { @@ -1852,8 +1854,16 @@ expandRTE(RangeTblEntry *rte, int rtindex, int sublevels_up, *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, - copyObject(avar)); + makeNullConst(INT4OID, -1, + InvalidOid)); + } } continue; } @@ -2242,6 +2252,7 @@ get_rte_attribute_type(RangeTblEntry *rte, AttrNumber attnum, 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); @@ -2304,7 +2315,7 @@ get_rte_attribute_is_dropped(RangeTblEntry *rte, AttrNumber attnum) * 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. + * signaled to us by a null pointer in the joinaliasvars list. */ Var *aliasvar; @@ -2313,7 +2324,7 @@ get_rte_attribute_is_dropped(RangeTblEntry *rte, AttrNumber attnum) elog(ERROR, "invalid varattno %d", attnum); aliasvar = (Var *) list_nth(rte->joinaliasvars, attnum - 1); - result = IsA(aliasvar, Const); + result = (aliasvar == NULL); } break; case RTE_FUNCTION: diff --git a/src/backend/parser/parse_target.c b/src/backend/parser/parse_target.c index ca20e77ce6..9c6c202c8e 100644 --- a/src/backend/parser/parse_target.c +++ b/src/backend/parser/parse_target.c @@ -311,6 +311,7 @@ markTargetListOrigin(ParseState *pstate, TargetEntry *tle, 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; @@ -1461,6 +1462,8 @@ expandRecordVariable(ParseState *pstate, Var *var, int levelsup) /* 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 7f527bd74a..3c7974adc7 100644 --- a/src/backend/rewrite/rewriteHandler.c +++ b/src/backend/rewrite/rewriteHandler.c @@ -91,9 +91,8 @@ static Query *fireRIRrules(Query *parsetree, List *activeRIRs, * 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. + * 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. @@ -160,8 +159,8 @@ AcquireRewriteLocks(Query *parsetree, bool forUpdatePushedDown) /* * Scan the join's alias var list to see if any columns have - * been dropped, and if so replace those Vars with NULL - * Consts. + * 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 @@ -172,16 +171,20 @@ AcquireRewriteLocks(Query *parsetree, bool forUpdatePushedDown) curinputrte = NULL; foreach(ll, rte->joinaliasvars) { - Var *aliasvar = (Var *) lfirst(ll); + 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 - * constant already? But that's OK too.) + * the join clause. (Conceivably it could also be a null + * pointer already? But that's OK too.) */ - if (IsA(aliasvar, Var)) + if (aliasvar && IsA(aliasvar, Var)) { /* * The elements of an alias list have to refer to @@ -205,15 +208,11 @@ AcquireRewriteLocks(Query *parsetree, bool forUpdatePushedDown) 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); + /* Replace the join alias item with a NULL */ + aliasitem = NULL; } } - newaliasvars = lappend(newaliasvars, aliasvar); + 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 40b565a11d..cecb542727 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -235,6 +235,7 @@ typedef struct * 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, @@ -3053,6 +3054,13 @@ set_join_column_names(deparse_namespace *dpns, RangeTblEntry *rte, 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]; @@ -3061,15 +3069,9 @@ set_join_column_names(deparse_namespace *dpns, RangeTblEntry *rte, 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; + 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) @@ -3402,7 +3404,14 @@ identify_join_columns(JoinExpr *j, RangeTblEntry *jrte, { Var *aliasvar = (Var *) lfirst(lc); - if (IsA(aliasvar, Var)) + /* 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); @@ -3422,15 +3431,8 @@ identify_join_columns(JoinExpr *j, RangeTblEntry *jrte, */ } 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++; } @@ -5359,7 +5361,8 @@ get_variable(Var *var, int levelsup, bool istoplevel, deparse_context *context) Var *aliasvar; aliasvar = (Var *) list_nth(rte->joinaliasvars, attnum - 1); - if (IsA(aliasvar, Var)) + /* we intentionally don't strip implicit coercions here */ + if (aliasvar && IsA(aliasvar, Var)) { return get_variable(aliasvar, var->varlevelsup + levelsup, istoplevel, context); @@ -5670,6 +5673,8 @@ get_name_for_var_field(Var *var, int fieldno, 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 9415e2c636..b4013e893d 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -660,7 +660,7 @@ typedef struct XmlSerialize * 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 + * 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.) * @@ -731,13 +731,19 @@ typedef struct RangeTblEntry /* * 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. + * 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 4fa7749503..8b45142967 100644 --- a/src/test/regress/expected/create_view.out +++ b/src/test/regress/expected/create_view.out @@ -1243,6 +1243,33 @@ select pg_get_viewdef('vv4', true); 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 3d85d9cfdc..4fbd5a5e6f 100644 --- a/src/test/regress/sql/create_view.sql +++ b/src/test/regress/sql/create_view.sql @@ -389,6 +389,21 @@ select pg_get_viewdef('vv2', true); 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;