Standardize on the assumption that the arguments of a RowExpr correspond
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 17 Aug 2004 18:47:09 +0000 (18:47 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 17 Aug 2004 18:47:09 +0000 (18:47 +0000)
to the physical layout of the rowtype, ie, there are dummy arguments
corresponding to any dropped columns in the rowtype.  We formerly had a
couple of places that did it this way and several others that did not.
Fixes Gaetano Mendola's "cache lookup failed for type 0" bug of 5-Aug.

src/backend/executor/execQual.c
src/backend/optimizer/util/clauses.c
src/backend/parser/parse_coerce.c
src/backend/parser/parse_relation.c
src/backend/rewrite/rewriteManip.c
src/backend/utils/adt/ruleutils.c
src/include/nodes/primnodes.h
src/include/parser/parsetree.h

index 6ac61d3c5beb12389149bdea69e9a33ef7607fdf..7394b3d32a91075376eccd58764dae02a4be5615 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/executor/execQual.c,v 1.165 2004/08/02 01:30:41 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/executor/execQual.c,v 1.166 2004/08/17 18:47:08 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -44,6 +44,7 @@
 #include "executor/nodeSubplan.h"
 #include "funcapi.h"
 #include "miscadmin.h"
+#include "nodes/makefuncs.h"
 #include "optimizer/planmain.h"
 #include "parser/parse_expr.h"
 #include "utils/acl.h"
@@ -2096,7 +2097,7 @@ ExecEvalRow(RowExprState *rstate,
    HeapTuple   tuple;
    Datum      *values;
    char       *nulls;
-   int         nargs;
+   int         natts;
    ListCell   *arg;
    int         i;
 
@@ -2106,9 +2107,12 @@ ExecEvalRow(RowExprState *rstate,
        *isDone = ExprSingleResult;
 
    /* Allocate workspace */
-   nargs = list_length(rstate->args);
-   values = (Datum *) palloc(nargs * sizeof(Datum));
-   nulls = (char *) palloc(nargs * sizeof(char));
+   natts = rstate->tupdesc->natts;
+   values = (Datum *) palloc0(natts * sizeof(Datum));
+   nulls = (char *) palloc(natts * sizeof(char));
+
+   /* preset to nulls in case rowtype has some later-added columns */
+   memset(nulls, 'n', natts * sizeof(char));
 
    /* Evaluate field values */
    i = 0;
@@ -2979,19 +2983,12 @@ ExecInitExpr(Expr *node, PlanState *parent)
            {
                RowExpr    *rowexpr = (RowExpr *) node;
                RowExprState *rstate = makeNode(RowExprState);
+               Form_pg_attribute *attrs;
                List       *outlist = NIL;
                ListCell   *l;
+               int         i;
 
                rstate->xprstate.evalfunc = (ExprStateEvalFunc) ExecEvalRow;
-               foreach(l, rowexpr->args)
-               {
-                   Expr       *e = (Expr *) lfirst(l);
-                   ExprState  *estate;
-
-                   estate = ExecInitExpr(e, parent);
-                   outlist = lappend(outlist, estate);
-               }
-               rstate->args = outlist;
                /* Build tupdesc to describe result tuples */
                if (rowexpr->row_typeid == RECORDOID)
                {
@@ -3003,7 +3000,46 @@ ExecInitExpr(Expr *node, PlanState *parent)
                {
                    /* it's been cast to a named type, use that */
                    rstate->tupdesc = lookup_rowtype_tupdesc(rowexpr->row_typeid, -1);
+                   rstate->tupdesc = CreateTupleDescCopy(rstate->tupdesc);
                }
+               /* Set up evaluation, skipping any deleted columns */
+               Assert(list_length(rowexpr->args) <= rstate->tupdesc->natts);
+               attrs = rstate->tupdesc->attrs;
+               i = 0;
+               foreach(l, rowexpr->args)
+               {
+                   Expr       *e = (Expr *) lfirst(l);
+                   ExprState  *estate;
+
+                   if (!attrs[i]->attisdropped)
+                   {
+                       /*
+                        * Guard against ALTER COLUMN TYPE on rowtype
+                        * since the RowExpr was created.  XXX should we
+                        * check typmod too?  Not sure we can be sure it'll
+                        * be the same.
+                        */
+                       if (exprType((Node *) e) != attrs[i]->atttypid)
+                           ereport(ERROR,
+                                   (errcode(ERRCODE_DATATYPE_MISMATCH),
+                                    errmsg("ROW() column has type %s instead of type %s",
+                                           format_type_be(exprType((Node *) e)),
+                                           format_type_be(attrs[i]->atttypid))));
+                   }
+                   else
+                   {
+                       /*
+                        * Ignore original expression and insert a NULL.
+                        * We don't really care what type of NULL it is,
+                        * so always make an int4 NULL.
+                        */
+                       e = (Expr *) makeNullConst(INT4OID);
+                   }
+                   estate = ExecInitExpr(e, parent);
+                   outlist = lappend(outlist, estate);
+                   i++;
+               }
+               rstate->args = outlist;
                state = (ExprState *) rstate;
            }
            break;
index 68d2529889e86d7c884acb63c4ed8fdcb1a3a0e7..f941375127c4cd4397019b9c064193b375a5ae66 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/optimizer/util/clauses.c,v 1.177 2004/08/02 01:30:43 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/optimizer/util/clauses.c,v 1.178 2004/08/17 18:47:08 tgl Exp $
  *
  * HISTORY
  *   AUTHOR            DATE            MAJOR EVENT
@@ -40,6 +40,7 @@
 #include "utils/lsyscache.h"
 #include "utils/memutils.h"
 #include "utils/syscache.h"
+#include "utils/typcache.h"
 
 
 typedef struct
@@ -1054,6 +1055,33 @@ set_coercionform_dontcare_walker(Node *node, void *context)
                                  context);
 }
 
+/*
+ * Helper for eval_const_expressions: check that datatype of an attribute
+ * is still what it was when the expression was parsed.  This is needed to
+ * guard against improper simplification after ALTER COLUMN TYPE.  (XXX we
+ * may well need to make similar checks elsewhere?)
+ */
+static bool
+rowtype_field_matches(Oid rowtypeid, int fieldnum,
+                     Oid expectedtype, int32 expectedtypmod)
+{
+   TupleDesc   tupdesc;
+   Form_pg_attribute attr;
+
+   /* No issue for RECORD, since there is no way to ALTER such a type */
+   if (rowtypeid == RECORDOID)
+       return true;
+   tupdesc = lookup_rowtype_tupdesc(rowtypeid, -1);
+   if (fieldnum <= 0 || fieldnum > tupdesc->natts)
+       return false;
+   attr = tupdesc->attrs[fieldnum - 1];
+   if (attr->attisdropped ||
+       attr->atttypid != expectedtype ||
+       attr->atttypmod != expectedtypmod)
+       return false;
+   return true;
+}
+
 
 /*--------------------
  * eval_const_expressions
@@ -1630,6 +1658,10 @@ eval_const_expressions_mutator(Node *node,
         * parser, because ParseComplexProjection short-circuits it. But
         * it can arise while simplifying functions.)  Also, we can
         * optimize field selection from a RowExpr construct.
+        *
+        * We must however check that the declared type of the field is
+        * still the same as when the FieldSelect was created --- this
+        * can change if someone did ALTER COLUMN TYPE on the rowtype.
         */
        FieldSelect *fselect = (FieldSelect *) node;
        FieldSelect *newfselect;
@@ -1640,11 +1672,15 @@ eval_const_expressions_mutator(Node *node,
        if (arg && IsA(arg, Var) &&
            ((Var *) arg)->varattno == InvalidAttrNumber)
        {
-           return (Node *) makeVar(((Var *) arg)->varno,
-                                   fselect->fieldnum,
-                                   fselect->resulttype,
-                                   fselect->resulttypmod,
-                                   ((Var *) arg)->varlevelsup);
+           if (rowtype_field_matches(((Var *) arg)->vartype,
+                                     fselect->fieldnum,
+                                     fselect->resulttype,
+                                     fselect->resulttypmod))
+               return (Node *) makeVar(((Var *) arg)->varno,
+                                       fselect->fieldnum,
+                                       fselect->resulttype,
+                                       fselect->resulttypmod,
+                                       ((Var *) arg)->varlevelsup);
        }
        if (arg && IsA(arg, RowExpr))
        {
@@ -1652,7 +1688,18 @@ eval_const_expressions_mutator(Node *node,
 
            if (fselect->fieldnum > 0 &&
                fselect->fieldnum <= list_length(rowexpr->args))
-               return (Node *) list_nth(rowexpr->args, fselect->fieldnum - 1);
+           {
+               Node *fld = (Node *) list_nth(rowexpr->args,
+                                             fselect->fieldnum - 1);
+
+               if (rowtype_field_matches(rowexpr->row_typeid,
+                                         fselect->fieldnum,
+                                         fselect->resulttype,
+                                         fselect->resulttypmod) &&
+                   fselect->resulttype == exprType(fld) &&
+                   fselect->resulttypmod == exprTypmod(fld))
+                   return fld;
+           }
        }
        newfselect = makeNode(FieldSelect);
        newfselect->arg = (Expr *) arg;
index 3878d07d5a189f8be32b41bb0892b06676359e45..8065b261beb1935c7592d81c6c40843e31470b3c 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/parser/parse_coerce.c,v 2.119 2004/06/16 01:26:44 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/parser/parse_coerce.c,v 2.120 2004/08/17 18:47:08 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -648,10 +648,15 @@ coerce_record_to_complex(ParseState *pstate, Node *node,
    List       *args = NIL;
    List       *newargs;
    int         i;
+   int         ucolno;
    ListCell   *arg;
 
    if (node && IsA(node, RowExpr))
    {
+       /*
+        * Since the RowExpr must be of type RECORD, we needn't worry
+        * about it containing any dropped columns.
+        */
        args = ((RowExpr *) node)->args;
    }
    else if (node && IsA(node, Var) &&
@@ -670,6 +675,8 @@ coerce_record_to_complex(ParseState *pstate, Node *node,
            Oid     vartype;
            int32   vartypmod;
 
+           if (get_rte_attribute_is_dropped(rte, nf))
+               continue;
            get_rte_attribute_type(rte, nf, &vartype, &vartypmod);
            args = lappend(args,
                           makeVar(((Var *) node)->varno,
@@ -687,19 +694,34 @@ coerce_record_to_complex(ParseState *pstate, Node *node,
                        format_type_be(targetTypeId))));
 
    tupdesc = lookup_rowtype_tupdesc(targetTypeId, -1);
-   if (list_length(args) != tupdesc->natts)
-       ereport(ERROR,
-               (errcode(ERRCODE_CANNOT_COERCE),
-                errmsg("cannot cast type %s to %s",
-                       format_type_be(RECORDOID),
-                       format_type_be(targetTypeId)),
-                errdetail("Input has wrong number of columns.")));
    newargs = NIL;
-   i = 0;
-   foreach(arg, args)
+   ucolno = 1;
+   arg = list_head(args);
+   for (i = 0; i < tupdesc->natts; i++)
    {
-       Node   *expr = (Node *) lfirst(arg);
-       Oid     exprtype = exprType(expr);
+       Node   *expr;
+       Oid     exprtype;
+
+       /* Fill in NULLs for dropped columns in rowtype */
+       if (tupdesc->attrs[i]->attisdropped)
+       {
+           /*
+            * can't use atttypid here, but it doesn't really matter
+            * what type the Const claims to be.
+            */
+           newargs = lappend(newargs, makeNullConst(INT4OID));
+           continue;
+       }
+
+       if (arg == NULL)
+           ereport(ERROR,
+                   (errcode(ERRCODE_CANNOT_COERCE),
+                    errmsg("cannot cast type %s to %s",
+                           format_type_be(RECORDOID),
+                           format_type_be(targetTypeId)),
+                    errdetail("Input has too few columns.")));
+       expr = (Node *) lfirst(arg);
+       exprtype = exprType(expr);
 
        expr = coerce_to_target_type(pstate,
                                     expr, exprtype,
@@ -716,10 +738,18 @@ coerce_record_to_complex(ParseState *pstate, Node *node,
                     errdetail("Cannot cast type %s to %s in column %d.",
                               format_type_be(exprtype),
                               format_type_be(tupdesc->attrs[i]->atttypid),
-                              i + 1)));
+                              ucolno)));
        newargs = lappend(newargs, expr);
-       i++;
+       ucolno++;
+       arg = lnext(arg);
    }
+   if (arg != NULL)
+       ereport(ERROR,
+               (errcode(ERRCODE_CANNOT_COERCE),
+                errmsg("cannot cast type %s to %s",
+                       format_type_be(RECORDOID),
+                       format_type_be(targetTypeId)),
+                errdetail("Input has too many columns.")));
 
    rowexpr = makeNode(RowExpr);
    rowexpr->args = newargs;
index fcec2cb39dd3de58274e3ea38afc4845a486fb2f..3f32f8c80f5340f7aac15270e9b05e96a142f560 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/parser/parse_relation.c,v 1.96 2004/05/30 23:40:35 neilc Exp $
+ *   $PostgreSQL: pgsql/src/backend/parser/parse_relation.c,v 1.97 2004/08/17 18:47:08 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -42,8 +42,6 @@ static Node *scanNameSpaceForRelid(ParseState *pstate, Node *nsnode,
 static void scanNameSpaceForConflict(ParseState *pstate, Node *nsnode,
                         RangeTblEntry *rte1, const char *aliasname1);
 static bool isForUpdate(ParseState *pstate, char *refname);
-static bool get_rte_attribute_is_dropped(RangeTblEntry *rte,
-                            AttrNumber attnum);
 static int specialAttNum(const char *attname);
 static void warnAutoRange(ParseState *pstate, RangeVar *relation);
 
@@ -1699,7 +1697,7 @@ get_rte_attribute_type(RangeTblEntry *rte, AttrNumber attnum,
  * get_rte_attribute_is_dropped
  *     Check whether attempted attribute ref is to a dropped column
  */
-static bool
+bool
 get_rte_attribute_is_dropped(RangeTblEntry *rte, AttrNumber attnum)
 {
    bool        result;
index d8cd7de5efa7ca4ffb0a39298eb46f74212227af..86412e90634adf34a0c47e82c09c23d0d1d39320 100644 (file)
@@ -7,12 +7,13 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/rewrite/rewriteManip.c,v 1.84 2004/05/30 23:40:35 neilc Exp $
+ *   $PostgreSQL: pgsql/src/backend/rewrite/rewriteManip.c,v 1.85 2004/08/17 18:47:09 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
 #include "postgres.h"
 
+#include "catalog/pg_type.h"
 #include "nodes/makefuncs.h"
 #include "optimizer/clauses.h"
 #include "optimizer/tlist.h"
@@ -938,18 +939,30 @@ ResolveNew_mutator(Node *node, ResolveNew_context *context)
 
                for (nf = 1; nf <= nfields; nf++)
                {
-                   Oid     vartype;
-                   int32   vartypmod;
-                   Var    *newvar;
-
-                   get_rte_attribute_type(rte, nf, &vartype, &vartypmod);
-                   newvar = makeVar(this_varno,
-                                    nf,
-                                    vartype,
-                                    vartypmod,
-                                    this_varlevelsup);
-                   fields = lappend(fields,
-                                    resolve_one_var(newvar, context));
+                   if (get_rte_attribute_is_dropped(rte, nf))
+                   {
+                       /*
+                        * can't determine att type here, but it doesn't
+                        * really matter what type the Const claims to be.
+                        */
+                       fields = lappend(fields,
+                                        makeNullConst(INT4OID));
+                   }
+                   else
+                   {
+                       Oid     vartype;
+                       int32   vartypmod;
+                       Var    *newvar;
+
+                       get_rte_attribute_type(rte, nf, &vartype, &vartypmod);
+                       newvar = makeVar(this_varno,
+                                        nf,
+                                        vartype,
+                                        vartypmod,
+                                        this_varlevelsup);
+                       fields = lappend(fields,
+                                        resolve_one_var(newvar, context));
+                   }
                }
 
                rowexpr = makeNode(RowExpr);
index e3b9a4e068a9f62f127d93131e166bbc256e4094..8e1420d9267b037212034c5d83a08a96f32be277 100644 (file)
@@ -3,7 +3,7 @@
  *             back to source text
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/utils/adt/ruleutils.c,v 1.176 2004/08/02 04:27:15 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/utils/adt/ruleutils.c,v 1.177 2004/08/17 18:47:09 tgl Exp $
  *
  *   This software is copyrighted by Jan Wieck - Hamburg.
  *
@@ -3283,22 +3283,54 @@ get_rule_expr(Node *node, deparse_context *context,
        case T_RowExpr:
            {
                RowExpr    *rowexpr = (RowExpr *) node;
+               TupleDesc   tupdesc = NULL;
                ListCell   *arg;
+               int         i;
                char       *sep;
 
                /*
-                * SQL99 allows "ROW" to be omitted when list_length(args) > 1,
-                * but for simplicity we always print it.
+                * If it's a named type and not RECORD, we may have to skip
+                * dropped columns and/or claim there are NULLs for added
+                * columns.
+                */
+               if (rowexpr->row_typeid != RECORDOID)
+               {
+                   tupdesc = lookup_rowtype_tupdesc(rowexpr->row_typeid, -1);
+                   Assert(list_length(rowexpr->args) <= tupdesc->natts);
+               }
+
+               /*
+                * SQL99 allows "ROW" to be omitted when there is more than
+                * one column, but for simplicity we always print it.
                 */
                appendStringInfo(buf, "ROW(");
                sep = "";
+               i = 0;
                foreach(arg, rowexpr->args)
                {
                    Node       *e = (Node *) lfirst(arg);
 
-                   appendStringInfo(buf, sep);
-                   get_rule_expr(e, context, true);
-                   sep = ", ";
+                   if (tupdesc == NULL ||
+                       !tupdesc->attrs[i]->attisdropped)
+                   {
+                       appendStringInfo(buf, sep);
+                       get_rule_expr(e, context, true);
+                       sep = ", ";
+                   }
+                   i++;
+               }
+               if (tupdesc != NULL)
+               {
+                   while (i < tupdesc->natts)
+                   {
+                       if (!tupdesc->attrs[i]->attisdropped)
+                       {
+                           appendStringInfo(buf, sep);
+                           appendStringInfo(buf, "NULL");
+                           sep = ", ";
+                       }
+                       i++;
+                   }
                }
                appendStringInfo(buf, ")");
                if (rowexpr->row_format == COERCE_EXPLICIT_CAST)
index 3ed4d74ee3546cb3d270650f23320805bb5e0c89..5abdbd548fd3fc40e80e8e06fdbe6e5e573e2ccf 100644 (file)
@@ -10,7 +10,7 @@
  * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/nodes/primnodes.h,v 1.100 2004/06/09 19:08:18 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/nodes/primnodes.h,v 1.101 2004/08/17 18:47:09 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -97,13 +97,16 @@ typedef struct Resdom
  * Alias -
  *   specifies an alias for a range variable; the alias might also
  *   specify renaming of columns within the table.
+ *
+ * Note: colnames is a list of Value nodes (always strings).  In an RTE's
+ * eref Alias, the colnames list includes dropped columns, so that the
+ * colname list position matches the physical attribute number.
  */
 typedef struct Alias
 {
    NodeTag     type;
    char       *aliasname;      /* aliased rel name (never qualified) */
    List       *colnames;       /* optional list of column aliases */
-   /* Note: colnames is a list of Value nodes (always strings) */
 } Alias;
 
 typedef enum InhOption
@@ -663,6 +666,16 @@ typedef struct ArrayExpr
 
 /*
  * RowExpr - a ROW() expression
+ *
+ * Note: the list of fields must have a one-for-one correspondence with
+ * physical fields of the associated rowtype, although it is okay for it
+ * to be shorter than the rowtype.  That is, the N'th list element must
+ * match up with the N'th physical field.  When the N'th physical field
+ * is a dropped column (attisdropped) then the N'th list element can just
+ * be a NULL constant.  (This case can only occur for named composite types,
+ * not RECORD types, since those are built from the RowExpr itself rather
+ * than vice versa.)  It is important not to assume that length(args) is
+ * the same as the number of columns logically present in the rowtype.
  */
 typedef struct RowExpr
 {
index 9b488c397ac7c625e9583de2b110de73b04e023a..e7f401ac315c896d4a12d4b17b31c019898c442b 100644 (file)
@@ -8,7 +8,7 @@
  * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/parser/parsetree.h,v 1.24 2004/05/26 04:41:46 neilc Exp $
+ * $PostgreSQL: pgsql/src/include/parser/parsetree.h,v 1.25 2004/08/17 18:47:09 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -55,6 +55,13 @@ extern char *get_rte_attribute_name(RangeTblEntry *rte, AttrNumber attnum);
 extern void get_rte_attribute_type(RangeTblEntry *rte, AttrNumber attnum,
                       Oid *vartype, int32 *vartypmod);
 
+/*
+ * Check whether an attribute of an RTE has been dropped (note that
+ * get_rte_attribute_type will fail on such an attr)
+ */
+extern bool get_rte_attribute_is_dropped(RangeTblEntry *rte,
+                                        AttrNumber attnum);
+
 
 /* ----------------
  *     target list operations