]> granicus.if.org Git - postgresql/commitdiff
Change post-rewriter representation of dropped columns in joinaliasvars.
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 23 Jul 2013 20:23:04 +0000 (16:23 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 23 Jul 2013 20:23:52 +0000 (16:23 -0400)
It's possible to drop a column from an input table of a JOIN clause in a
view, if that column is nowhere actually referenced in the view.  But it
will still be there in the JOIN clause's joinaliasvars list.  We used to
replace such entries with NULL Const nodes, which is handy for generation
of RowExpr expansion of a whole-row reference to the view.  The trouble
with that is that it can't be distinguished from the situation after
subquery pull-up of a constant subquery output expression below the JOIN.
Instead, replace such joinaliasvars with null pointers (empty expression
trees), which can't be confused with pulled-up expressions.  expandRTE()
still emits the old convention, though, for convenience of RowExpr
generation and to reduce the risk of breaking extension code.

In HEAD and 9.3, this patch also fixes a problem with some new code in
ruleutils.c that was failing to cope with implicitly-casted joinaliasvars
entries, as per recent report from Feike Steenbergen.  That oversight was
because of an inadequate description of the data structure in parsenodes.h,
which I've now corrected.  There were some pre-existing oversights of the
same ilk elsewhere, which I believe are now all fixed.

src/backend/optimizer/util/var.c
src/backend/parser/parse_relation.c
src/backend/parser/parse_target.c
src/backend/rewrite/rewriteHandler.c
src/backend/utils/adt/ruleutils.c
src/include/nodes/parsenodes.h
src/test/regress/expected/create_view.out
src/test/regress/sql/create_view.sql

index 7eaf8d27bf08bf5dd1776d203876adb8396c73b3..5f736ad6c4060ff4d7dabb3844a04185e01fa3ef 100644 (file)
@@ -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);
 
                /*
index a9254c8c3a2e33b7c293ef51c53c78a797b1d4f1..42de89f510190877b1f6fa357efb08c81eb7acc9 100644 (file)
@@ -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:
index ca20e77ce6d1b09ff0a2012f7d3084d33c40543d..9c6c202c8e6ef16a981fdafa8945741ca70cd709 100644 (file)
@@ -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 */
index 4163301542ab4537667cc3b270ea5ca645441973..f8e261a7b90afb218b2e1823291bfade16f2e5ea 100644 (file)
@@ -19,6 +19,7 @@
 #include "foreign/fdwapi.h"
 #include "nodes/makefuncs.h"
 #include "nodes/nodeFuncs.h"
+#include "optimizer/clauses.h"
 #include "parser/analyze.h"
 #include "parser/parse_coerce.h"
 #include "parser/parsetree.h"
@@ -90,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.
@@ -159,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
@@ -171,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
@@ -204,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;
index a1ed7813f24f58fff2fb2fed8d13a005bc248d9c..628b83a67f6da4cad394cde6a52ec53b8da04862 100644 (file)
@@ -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,
index 6723647e2e397126e6e2ba480068fd40748f1e59..0eac9fb97e293a1d02c356b3b0531327361b37d6 100644 (file)
@@ -652,7 +652,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.)
  *
@@ -723,13 +723,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 */
index 11ac795677858074c7617102d07aaa9874148240..3ca8a16ec86b1005fc40f075ec0c3345e5e5d8fd 100644 (file)
@@ -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;
index 3d85d9cfdc50b454fb38d8a742770eda1e119a41..4fbd5a5e6f8baa3d41f1a201fbb99ad4bc3bf3da 100644 (file)
@@ -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;