Re: BUG #13907: Restore materialized view throw permission denied - Mailing list pgsql-bugs

Michael Paquier <michael.paquier@gmail.com> writes:
> Attached is a new patch, with the promised refactoring, more
> regression tests, etc. After some thoughts, I have arrived to the
> conclusion that it is better to limit the footprint of this patch in
> views.c. So I have created a routine makeColumnDef that is used for
> views, ctas and matviews, but I am letting the creation of the column
> definition list separated as each code path has slight differences
> when building it. Things could be made more shared on HEAD but that
> would be really intrusive for back branches, and I have kept that in
> mind for this patch.

I'm planning to apply the attached revision as soon as I get done
back-porting it.  Main differences from your version:

* We can, and I think should, skip the rewriting phase too in the WITH NO
DATA case.  Rewriting should never change a query's exposed result
rowtype, and any other side-effects it might have are likely to be bad
for our purposes.

* Rather than add a goto, I put the existing code sequence into the if's
else block.  This will probably cause me some back-patching pain, but
I don't like uglifying code just to make back-patch simpler.

* The regression test cases you added seem not entirely on point, because
they pass just fine against HEAD.  I don't object to them, but I added
this to exercise the desired behavior change:

-- make sure that create WITH NO DATA does not plan the query (bug #13907)
create materialized view mvtest_error as select 1/0 as x;  -- fail
create materialized view mvtest_error as select 1/0 as x with no data;
refresh materialized view mvtest_error;  -- fail here
drop materialized view mvtest_error;

* I also got rid of the static variable CreateAsReladdr, which while
not related to the immediate problem is ugly and dangerous.  (It'd
cause a failure in a nested-CREATE-TABLE-AS situation, which would
be unusual but surely isn't forbidden.)


I spent a little bit of time wondering whether we couldn't get rid of
having intorel_startup create the relation at all, instead always doing
it in ExecCreateTableAs().  But that doesn't work conveniently for
CREATE TABLE AS EXECUTE, since there's no query tree handy in that case.
You could imagine moving the build-ColumnDefs-from-a-TupleDesc logic
to someplace else that's only used for CREATE TABLE AS EXECUTE, but
it's not clear to me that it's worth further refactoring just to make
the WITH DATA and WITH NO DATA cases a bit more alike.

            regards, tom lane

diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index 5a853c4..5b4f6af 100644
*** a/src/backend/commands/createas.c
--- b/src/backend/commands/createas.c
***************
*** 10,16 ****
   *
   * Formerly, CTAS was implemented as a variant of SELECT, which led
   * to assorted legacy behaviors that we still try to preserve, notably that
!  * we must return a tuples-processed count in the completionTag.
   *
   * Portions Copyright (c) 1996-2016, PostgreSQL Global Development Group
   * Portions Copyright (c) 1994, Regents of the University of California
--- 10,17 ----
   *
   * Formerly, CTAS was implemented as a variant of SELECT, which led
   * to assorted legacy behaviors that we still try to preserve, notably that
!  * we must return a tuples-processed count in the completionTag.  (We no
!  * longer do that for CTAS ... WITH NO DATA, however.)
   *
   * Portions Copyright (c) 1996-2016, PostgreSQL Global Development Group
   * Portions Copyright (c) 1994, Regents of the University of California
***************
*** 36,41 ****
--- 37,44 ----
  #include "commands/tablecmds.h"
  #include "commands/view.h"
  #include "miscadmin.h"
+ #include "nodes/makefuncs.h"
+ #include "nodes/nodeFuncs.h"
  #include "parser/parse_clause.h"
  #include "rewrite/rewriteHandler.h"
  #include "storage/smgr.h"
*************** typedef struct
*** 53,66 ****
      IntoClause *into;            /* target relation specification */
      /* These fields are filled by intorel_startup: */
      Relation    rel;            /* relation to write to */
      CommandId    output_cid;        /* cmin to insert in output tuples */
      int            hi_options;        /* heap_insert performance options */
      BulkInsertState bistate;    /* bulk insert state */
  } DR_intorel;

! /* the address of the created table, for ExecCreateTableAs consumption */
! static ObjectAddress CreateAsReladdr = {InvalidOid, InvalidOid, 0};

  static void intorel_startup(DestReceiver *self, int operation, TupleDesc typeinfo);
  static bool intorel_receive(TupleTableSlot *slot, DestReceiver *self);
  static void intorel_shutdown(DestReceiver *self);
--- 56,72 ----
      IntoClause *into;            /* target relation specification */
      /* These fields are filled by intorel_startup: */
      Relation    rel;            /* relation to write to */
+     ObjectAddress reladdr;        /* address of rel, for ExecCreateTableAs */
      CommandId    output_cid;        /* cmin to insert in output tuples */
      int            hi_options;        /* heap_insert performance options */
      BulkInsertState bistate;    /* bulk insert state */
  } DR_intorel;

! /* utility functions for CTAS definition creation */
! static ObjectAddress create_ctas_internal(List *attrList, IntoClause *into);
! static ObjectAddress create_ctas_nodata(List *tlist, IntoClause *into);

+ /* DestReceiver routines for collecting data */
  static void intorel_startup(DestReceiver *self, int operation, TupleDesc typeinfo);
  static bool intorel_receive(TupleTableSlot *slot, DestReceiver *self);
  static void intorel_shutdown(DestReceiver *self);
*************** static void intorel_destroy(DestReceiver
*** 68,73 ****
--- 74,223 ----


  /*
+  * create_ctas_internal
+  *
+  * Internal utility used for the creation of the definition of a relation
+  * created via CREATE TABLE AS or a materialized view.  Caller needs to
+  * provide a list of attributes (ColumnDef nodes).
+  */
+ static ObjectAddress
+ create_ctas_internal(List *attrList, IntoClause *into)
+ {
+     CreateStmt *create = makeNode(CreateStmt);
+     bool        is_matview;
+     char        relkind;
+     Datum        toast_options;
+     static char *validnsps[] = HEAP_RELOPT_NAMESPACES;
+     ObjectAddress intoRelationAddr;
+
+     /* This code supports both CREATE TABLE AS and CREATE MATERIALIZED VIEW */
+     is_matview = (into->viewQuery != NULL);
+     relkind = is_matview ? RELKIND_MATVIEW : RELKIND_RELATION;
+
+     /*
+      * Create the target relation by faking up a CREATE TABLE parsetree and
+      * passing it to DefineRelation.
+      */
+     create->relation = into->rel;
+     create->tableElts = attrList;
+     create->inhRelations = NIL;
+     create->ofTypename = NULL;
+     create->constraints = NIL;
+     create->options = into->options;
+     create->oncommit = into->onCommit;
+     create->tablespacename = into->tableSpaceName;
+     create->if_not_exists = false;
+
+     /*
+      * Create the relation.  (This will error out if there's an existing view,
+      * so we don't need more code to complain if "replace" is false.)
+      */
+     intoRelationAddr = DefineRelation(create, relkind, InvalidOid, NULL);
+
+     /*
+      * If necessary, create a TOAST table for the target table.  Note that
+      * NewRelationCreateToastTable ends with CommandCounterIncrement(), so
+      * that the TOAST table will be visible for insertion.
+      */
+     CommandCounterIncrement();
+
+     /* parse and validate reloptions for the toast table */
+     toast_options = transformRelOptions((Datum) 0,
+                                         create->options,
+                                         "toast",
+                                         validnsps,
+                                         true, false);
+
+     (void) heap_reloptions(RELKIND_TOASTVALUE, toast_options, true);
+
+     NewRelationCreateToastTable(intoRelationAddr.objectId, toast_options);
+
+     /* Create the "view" part of a materialized view. */
+     if (is_matview)
+     {
+         /* StoreViewQuery scribbles on tree, so make a copy */
+         Query       *query = (Query *) copyObject(into->viewQuery);
+
+         StoreViewQuery(intoRelationAddr.objectId, query, false);
+         CommandCounterIncrement();
+     }
+
+     return intoRelationAddr;
+ }
+
+
+ /*
+  * create_ctas_nodata
+  *
+  * Create CTAS or materialized view when WITH NO DATA is used, starting from
+  * the targetlist of the SELECT or view definition.
+  */
+ static ObjectAddress
+ create_ctas_nodata(List *tlist, IntoClause *into)
+ {
+     List       *attrList;
+     ListCell   *t,
+                *lc;
+
+     /*
+      * Build list of ColumnDefs from non-junk elements of the tlist.  If a
+      * column name list was specified in CREATE TABLE AS, override the column
+      * names in the query.  (Too few column names are OK, too many are not.)
+      */
+     attrList = NIL;
+     lc = list_head(into->colNames);
+     foreach(t, tlist)
+     {
+         TargetEntry *tle = (TargetEntry *) lfirst(t);
+
+         if (!tle->resjunk)
+         {
+             ColumnDef  *col;
+             char       *colname;
+
+             if (lc)
+             {
+                 colname = strVal(lfirst(lc));
+                 lc = lnext(lc);
+             }
+             else
+                 colname = tle->resname;
+
+             col = makeColumnDef(colname,
+                                 exprType((Node *) tle->expr),
+                                 exprTypmod((Node *) tle->expr),
+                                 exprCollation((Node *) tle->expr));
+
+             /*
+              * It's possible that the column is of a collatable type but the
+              * collation could not be resolved, so double-check.  (We must
+              * check this here because DefineRelation would adopt the type's
+              * default collation rather than complaining.)
+              */
+             if (!OidIsValid(col->collOid) &&
+                 type_is_collatable(col->typeName->typeOid))
+                 ereport(ERROR,
+                         (errcode(ERRCODE_INDETERMINATE_COLLATION),
+                          errmsg("no collation was derived for column \"%s\" with collatable type %s",
+                                 col->colname,
+                                 format_type_be(col->typeName->typeOid)),
+                          errhint("Use the COLLATE clause to set the collation explicitly.")));
+
+             attrList = lappend(attrList, col);
+         }
+     }
+
+     if (lc != NULL)
+         ereport(ERROR,
+                 (errcode(ERRCODE_SYNTAX_ERROR),
+                  errmsg("too many column names were specified")));
+
+     /* Create the relation definition using the ColumnDef list */
+     return create_ctas_internal(attrList, into);
+ }
+
+
+ /*
   * ExecCreateTableAs -- execute a CREATE TABLE AS command
   */
  ObjectAddress
*************** ExecCreateTableAs(CreateTableAsStmt *stm
*** 85,91 ****
      List       *rewritten;
      PlannedStmt *plan;
      QueryDesc  *queryDesc;
-     ScanDirection dir;

      if (stmt->if_not_exists)
      {
--- 235,240 ----
*************** ExecCreateTableAs(CreateTableAsStmt *stm
*** 121,128 ****
          Assert(!is_matview);    /* excluded by syntax */
          ExecuteQuery(estmt, into, queryString, params, dest, completionTag);

!         address = CreateAsReladdr;
!         CreateAsReladdr = InvalidObjectAddress;
          return address;
      }
      Assert(query->commandType == CMD_SELECT);
--- 270,278 ----
          Assert(!is_matview);    /* excluded by syntax */
          ExecuteQuery(estmt, into, queryString, params, dest, completionTag);

!         /* get object address that intorel_startup saved for us */
!         address = ((DR_intorel *) dest)->reladdr;
!
          return address;
      }
      Assert(query->commandType == CMD_SELECT);
*************** ExecCreateTableAs(CreateTableAsStmt *stm
*** 142,211 ****
          save_nestlevel = NewGUCNestLevel();
      }

!     /*
!      * Parse analysis was done already, but we still have to run the rule
!      * rewriter.  We do not do AcquireRewriteLocks: we assume the query either
!      * came straight from the parser, or suitable locks were acquired by
!      * plancache.c.
!      *
!      * Because the rewriter and planner tend to scribble on the input, we make
!      * a preliminary copy of the source querytree.  This prevents problems in
!      * the case that CTAS is in a portal or plpgsql function and is executed
!      * repeatedly.  (See also the same hack in EXPLAIN and PREPARE.)
!      */
!     rewritten = QueryRewrite((Query *) copyObject(query));

!     /* SELECT should never rewrite to more or less than one SELECT query */
!     if (list_length(rewritten) != 1)
!         elog(ERROR, "unexpected rewrite result for CREATE TABLE AS SELECT");
!     query = (Query *) linitial(rewritten);
!     Assert(query->commandType == CMD_SELECT);

!     /* plan the query */
!     plan = pg_plan_query(query, 0, params);

!     /*
!      * Use a snapshot with an updated command ID to ensure this query sees
!      * results of any previously executed queries.  (This could only matter if
!      * the planner executed an allegedly-stable function that changed the
!      * database contents, but let's do it anyway to be parallel to the EXPLAIN
!      * code path.)
!      */
!     PushCopiedSnapshot(GetActiveSnapshot());
!     UpdateActiveSnapshotCommandId();

!     /* Create a QueryDesc, redirecting output to our tuple receiver */
!     queryDesc = CreateQueryDesc(plan, queryString,
!                                 GetActiveSnapshot(), InvalidSnapshot,
!                                 dest, params, 0);

!     /* call ExecutorStart to prepare the plan for execution */
!     ExecutorStart(queryDesc, GetIntoRelEFlags(into));

!     /*
!      * Normally, we run the plan to completion; but if skipData is specified,
!      * just do tuple receiver startup and shutdown.
!      */
!     if (into->skipData)
!         dir = NoMovementScanDirection;
!     else
!         dir = ForwardScanDirection;

!     /* run the plan */
!     ExecutorRun(queryDesc, dir, 0L);

!     /* save the rowcount if we're given a completionTag to fill */
!     if (completionTag)
!         snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
!                  "SELECT " UINT64_FORMAT, queryDesc->estate->es_processed);

!     /* and clean up */
!     ExecutorFinish(queryDesc);
!     ExecutorEnd(queryDesc);

!     FreeQueryDesc(queryDesc);

!     PopActiveSnapshot();

      if (is_matview)
      {
--- 292,372 ----
          save_nestlevel = NewGUCNestLevel();
      }

!     if (into->skipData)
!     {
!         /*
!          * If WITH NO DATA was specified, do not go through the rewriter,
!          * planner and executor.  Just define the relation using a code path
!          * similar to CREATE VIEW.  This avoids dump/restore problems stemming
!          * from running the planner before all dependencies are set up.
!          */
!         address = create_ctas_nodata(query->targetList, into);
!     }
!     else
!     {
!         /*
!          * Parse analysis was done already, but we still have to run the rule
!          * rewriter.  We do not do AcquireRewriteLocks: we assume the query
!          * either came straight from the parser, or suitable locks were
!          * acquired by plancache.c.
!          *
!          * Because the rewriter and planner tend to scribble on the input, we
!          * make a preliminary copy of the source querytree.  This prevents
!          * problems in the case that CTAS is in a portal or plpgsql function
!          * and is executed repeatedly.  (See also the same hack in EXPLAIN and
!          * PREPARE.)
!          */
!         rewritten = QueryRewrite((Query *) copyObject(query));

!         /* SELECT should never rewrite to more or less than one SELECT query */
!         if (list_length(rewritten) != 1)
!             elog(ERROR, "unexpected rewrite result for %s",
!                  is_matview ? "CREATE MATERIALIZED VIEW" :
!                  "CREATE TABLE AS SELECT");
!         query = (Query *) linitial(rewritten);
!         Assert(query->commandType == CMD_SELECT);

!         /* plan the query */
!         plan = pg_plan_query(query, 0, params);

!         /*
!          * Use a snapshot with an updated command ID to ensure this query sees
!          * results of any previously executed queries.  (This could only
!          * matter if the planner executed an allegedly-stable function that
!          * changed the database contents, but let's do it anyway to be
!          * parallel to the EXPLAIN code path.)
!          */
!         PushCopiedSnapshot(GetActiveSnapshot());
!         UpdateActiveSnapshotCommandId();

!         /* Create a QueryDesc, redirecting output to our tuple receiver */
!         queryDesc = CreateQueryDesc(plan, queryString,
!                                     GetActiveSnapshot(), InvalidSnapshot,
!                                     dest, params, 0);

!         /* call ExecutorStart to prepare the plan for execution */
!         ExecutorStart(queryDesc, GetIntoRelEFlags(into));

!         /* run the plan to completion */
!         ExecutorRun(queryDesc, ForwardScanDirection, 0L);

!         /* save the rowcount if we're given a completionTag to fill */
!         if (completionTag)
!             snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
!                      "SELECT " UINT64_FORMAT,
!                      queryDesc->estate->es_processed);

!         /* get object address that intorel_startup saved for us */
!         address = ((DR_intorel *) dest)->reladdr;

!         /* and clean up */
!         ExecutorFinish(queryDesc);
!         ExecutorEnd(queryDesc);

!         FreeQueryDesc(queryDesc);

!         PopActiveSnapshot();
!     }

      if (is_matview)
      {
*************** ExecCreateTableAs(CreateTableAsStmt *stm
*** 216,224 ****
          SetUserIdAndSecContext(save_userid, save_sec_context);
      }

-     address = CreateAsReladdr;
-     CreateAsReladdr = InvalidObjectAddress;
-
      return address;
  }

--- 377,382 ----
*************** intorel_startup(DestReceiver *self, int
*** 287,300 ****
      IntoClause *into = myState->into;
      bool        is_matview;
      char        relkind;
!     CreateStmt *create;
      ObjectAddress intoRelationAddr;
      Relation    intoRelationDesc;
      RangeTblEntry *rte;
-     Datum        toast_options;
      ListCell   *lc;
      int            attnum;
-     static char *validnsps[] = HEAP_RELOPT_NAMESPACES;

      Assert(into != NULL);        /* else somebody forgot to set it */

--- 445,456 ----
      IntoClause *into = myState->into;
      bool        is_matview;
      char        relkind;
!     List       *attrList;
      ObjectAddress intoRelationAddr;
      Relation    intoRelationDesc;
      RangeTblEntry *rte;
      ListCell   *lc;
      int            attnum;

      Assert(into != NULL);        /* else somebody forgot to set it */

*************** intorel_startup(DestReceiver *self, int
*** 303,364 ****
      relkind = is_matview ? RELKIND_MATVIEW : RELKIND_RELATION;

      /*
-      * Create the target relation by faking up a CREATE TABLE parsetree and
-      * passing it to DefineRelation.
-      */
-     create = makeNode(CreateStmt);
-     create->relation = into->rel;
-     create->tableElts = NIL;    /* will fill below */
-     create->inhRelations = NIL;
-     create->ofTypename = NULL;
-     create->constraints = NIL;
-     create->options = into->options;
-     create->oncommit = into->onCommit;
-     create->tablespacename = into->tableSpaceName;
-     create->if_not_exists = false;
-
-     /*
       * Build column definitions using "pre-cooked" type and collation info. If
       * a column name list was specified in CREATE TABLE AS, override the
       * column names derived from the query.  (Too few column names are OK, too
       * many are not.)
       */
      lc = list_head(into->colNames);
      for (attnum = 0; attnum < typeinfo->natts; attnum++)
      {
          Form_pg_attribute attribute = typeinfo->attrs[attnum];
!         ColumnDef  *col = makeNode(ColumnDef);
!         TypeName   *coltype = makeNode(TypeName);

          if (lc)
          {
!             col->colname = strVal(lfirst(lc));
              lc = lnext(lc);
          }
          else
!             col->colname = NameStr(attribute->attname);
!         col->typeName = coltype;
!         col->inhcount = 0;
!         col->is_local = true;
!         col->is_not_null = false;
!         col->is_from_type = false;
!         col->storage = 0;
!         col->raw_default = NULL;
!         col->cooked_default = NULL;
!         col->collClause = NULL;
!         col->collOid = attribute->attcollation;
!         col->constraints = NIL;
!         col->fdwoptions = NIL;
!         col->location = -1;

!         coltype->names = NIL;
!         coltype->typeOid = attribute->atttypid;
!         coltype->setof = false;
!         coltype->pct_type = false;
!         coltype->typmods = NIL;
!         coltype->typemod = attribute->atttypmod;
!         coltype->arrayBounds = NIL;
!         coltype->location = -1;

          /*
           * It's possible that the column is of a collatable type but the
--- 459,489 ----
      relkind = is_matview ? RELKIND_MATVIEW : RELKIND_RELATION;

      /*
       * Build column definitions using "pre-cooked" type and collation info. If
       * a column name list was specified in CREATE TABLE AS, override the
       * column names derived from the query.  (Too few column names are OK, too
       * many are not.)
       */
+     attrList = NIL;
      lc = list_head(into->colNames);
      for (attnum = 0; attnum < typeinfo->natts; attnum++)
      {
          Form_pg_attribute attribute = typeinfo->attrs[attnum];
!         ColumnDef  *col;
!         char       *colname;

          if (lc)
          {
!             colname = strVal(lfirst(lc));
              lc = lnext(lc);
          }
          else
!             colname = NameStr(attribute->attname);

!         col = makeColumnDef(colname,
!                             attribute->atttypid,
!                             attribute->atttypmod,
!                             attribute->attcollation);

          /*
           * It's possible that the column is of a collatable type but the
*************** intorel_startup(DestReceiver *self, int
*** 367,380 ****
           * collation rather than complaining.)
           */
          if (!OidIsValid(col->collOid) &&
!             type_is_collatable(coltype->typeOid))
              ereport(ERROR,
                      (errcode(ERRCODE_INDETERMINATE_COLLATION),
                       errmsg("no collation was derived for column \"%s\" with collatable type %s",
!                             col->colname, format_type_be(coltype->typeOid)),
                       errhint("Use the COLLATE clause to set the collation explicitly.")));

!         create->tableElts = lappend(create->tableElts, col);
      }

      if (lc != NULL)
--- 492,506 ----
           * collation rather than complaining.)
           */
          if (!OidIsValid(col->collOid) &&
!             type_is_collatable(col->typeName->typeOid))
              ereport(ERROR,
                      (errcode(ERRCODE_INDETERMINATE_COLLATION),
                       errmsg("no collation was derived for column \"%s\" with collatable type %s",
!                             col->colname,
!                             format_type_be(col->typeName->typeOid)),
                       errhint("Use the COLLATE clause to set the collation explicitly.")));

!         attrList = lappend(attrList, col);
      }

      if (lc != NULL)
*************** intorel_startup(DestReceiver *self, int
*** 385,419 ****
      /*
       * Actually create the target table
       */
!     intoRelationAddr = DefineRelation(create, relkind, InvalidOid, NULL);
!
!     /*
!      * If necessary, create a TOAST table for the target table.  Note that
!      * NewRelationCreateToastTable ends with CommandCounterIncrement(), so
!      * that the TOAST table will be visible for insertion.
!      */
!     CommandCounterIncrement();
!
!     /* parse and validate reloptions for the toast table */
!     toast_options = transformRelOptions((Datum) 0,
!                                         create->options,
!                                         "toast",
!                                         validnsps,
!                                         true, false);
!
!     (void) heap_reloptions(RELKIND_TOASTVALUE, toast_options, true);
!
!     NewRelationCreateToastTable(intoRelationAddr.objectId, toast_options);
!
!     /* Create the "view" part of a materialized view. */
!     if (is_matview)
!     {
!         /* StoreViewQuery scribbles on tree, so make a copy */
!         Query       *query = (Query *) copyObject(into->viewQuery);
!
!         StoreViewQuery(intoRelationAddr.objectId, query, false);
!         CommandCounterIncrement();
!     }

      /*
       * Finally we can open the target table
--- 511,517 ----
      /*
       * Actually create the target table
       */
!     intoRelationAddr = create_ctas_internal(attrList, into);

      /*
       * Finally we can open the target table
*************** intorel_startup(DestReceiver *self, int
*** 462,472 ****
       * Fill private fields of myState for use by later routines
       */
      myState->rel = intoRelationDesc;
      myState->output_cid = GetCurrentCommandId(true);

-     /* and remember the new relation's address for ExecCreateTableAs */
-     CreateAsReladdr = intoRelationAddr;
-
      /*
       * We can skip WAL-logging the insertions, unless PITR or streaming
       * replication is in use. We can skip the FSM in any case.
--- 560,568 ----
       * Fill private fields of myState for use by later routines
       */
      myState->rel = intoRelationDesc;
+     myState->reladdr = intoRelationAddr;
      myState->output_cid = GetCurrentCommandId(true);

      /*
       * We can skip WAL-logging the insertions, unless PITR or streaming
       * replication is in use. We can skip the FSM in any case.
diff --git a/src/backend/commands/view.c b/src/backend/commands/view.c
index e9d9ba2..085bf32 100644
*** a/src/backend/commands/view.c
--- b/src/backend/commands/view.c
*************** DefineVirtualRelation(RangeVar *relation
*** 82,106 ****
      attrList = NIL;
      foreach(t, tlist)
      {
!         TargetEntry *tle = lfirst(t);

          if (!tle->resjunk)
          {
!             ColumnDef  *def = makeNode(ColumnDef);
!
!             def->colname = pstrdup(tle->resname);
!             def->typeName = makeTypeNameFromOid(exprType((Node *) tle->expr),
!                                              exprTypmod((Node *) tle->expr));
!             def->inhcount = 0;
!             def->is_local = true;
!             def->is_not_null = false;
!             def->is_from_type = false;
!             def->storage = 0;
!             def->raw_default = NULL;
!             def->cooked_default = NULL;
!             def->collClause = NULL;
!             def->collOid = exprCollation((Node *) tle->expr);
!             def->location = -1;

              /*
               * It's possible that the column is of a collatable type but the
--- 82,95 ----
      attrList = NIL;
      foreach(t, tlist)
      {
!         TargetEntry *tle = (TargetEntry *) lfirst(t);

          if (!tle->resjunk)
          {
!             ColumnDef  *def = makeColumnDef(tle->resname,
!                                             exprType((Node *) tle->expr),
!                                             exprTypmod((Node *) tle->expr),
!                                           exprCollation((Node *) tle->expr));

              /*
               * It's possible that the column is of a collatable type but the
*************** DefineVirtualRelation(RangeVar *relation
*** 117,123 ****
              }
              else
                  Assert(!OidIsValid(def->collOid));
-             def->constraints = NIL;

              attrList = lappend(attrList, def);
          }
--- 106,111 ----
diff --git a/src/backend/nodes/makefuncs.c b/src/backend/nodes/makefuncs.c
index f625c32..d72a85e 100644
*** a/src/backend/nodes/makefuncs.c
--- b/src/backend/nodes/makefuncs.c
*************** makeTypeNameFromOid(Oid typeOid, int32 t
*** 477,482 ****
--- 477,512 ----
  }

  /*
+  * makeColumnDef -
+  *    build a ColumnDef node to represent a simple column definition.
+  *
+  * Type and collation are specified by OID.
+  * Other properties are all basic to start with.
+  */
+ ColumnDef *
+ makeColumnDef(const char *colname, Oid typeOid, int32 typmod, Oid collOid)
+ {
+     ColumnDef  *n = makeNode(ColumnDef);
+
+     n->colname = pstrdup(colname);
+     n->typeName = makeTypeNameFromOid(typeOid, typmod);
+     n->inhcount = 0;
+     n->is_local = true;
+     n->is_not_null = false;
+     n->is_from_type = false;
+     n->storage = 0;
+     n->raw_default = NULL;
+     n->cooked_default = NULL;
+     n->collClause = NULL;
+     n->collOid = collOid;
+     n->constraints = NIL;
+     n->fdwoptions = NIL;
+     n->location = -1;
+
+     return n;
+ }
+
+ /*
   * makeFuncExpr -
   *    build an expression tree representing a function call.
   *
diff --git a/src/include/nodes/makefuncs.h b/src/include/nodes/makefuncs.h
index ef34a6e..01c5cb4 100644
*** a/src/include/nodes/makefuncs.h
--- b/src/include/nodes/makefuncs.h
*************** extern TypeName *makeTypeName(char *typn
*** 72,77 ****
--- 72,80 ----
  extern TypeName *makeTypeNameFromNameList(List *names);
  extern TypeName *makeTypeNameFromOid(Oid typeOid, int32 typmod);

+ extern ColumnDef *makeColumnDef(const char *colname,
+               Oid typeOid, int32 typmod, Oid collOid);
+
  extern FuncExpr *makeFuncExpr(Oid funcid, Oid rettype, List *args,
               Oid funccollid, Oid inputcollid, CoercionForm fformat);

diff --git a/src/test/regress/expected/matview.out b/src/test/regress/expected/matview.out
index 7f9741e..102bf1f 100644
*** a/src/test/regress/expected/matview.out
--- b/src/test/regress/expected/matview.out
*************** DROP TABLE mvtest_boxes CASCADE;
*** 455,467 ****
  NOTICE:  drop cascades to materialized view mvtest_boxmv
  -- make sure that column names are handled correctly
  CREATE TABLE mvtest_v (i int, j int);
! CREATE MATERIALIZED VIEW mvtest_mv_v (ii) AS SELECT i, j AS jj FROM mvtest_v;
  ALTER TABLE mvtest_v RENAME COLUMN i TO x;
  INSERT INTO mvtest_v values (1, 2);
  CREATE UNIQUE INDEX mvtest_mv_v_ii ON mvtest_mv_v (ii);
  REFRESH MATERIALIZED VIEW mvtest_mv_v;
  UPDATE mvtest_v SET j = 3 WHERE x = 1;
  REFRESH MATERIALIZED VIEW CONCURRENTLY mvtest_mv_v;
  SELECT * FROM mvtest_v;
   x | j
  ---+---
--- 455,477 ----
  NOTICE:  drop cascades to materialized view mvtest_boxmv
  -- make sure that column names are handled correctly
  CREATE TABLE mvtest_v (i int, j int);
! CREATE MATERIALIZED VIEW mvtest_mv_v (ii, jj, kk) AS SELECT i, j FROM mvtest_v; -- error
! ERROR:  too many column names were specified
! CREATE MATERIALIZED VIEW mvtest_mv_v (ii, jj) AS SELECT i, j FROM mvtest_v; -- ok
! CREATE MATERIALIZED VIEW mvtest_mv_v_2 (ii) AS SELECT i, j FROM mvtest_v; -- ok
! CREATE MATERIALIZED VIEW mvtest_mv_v_3 (ii, jj, kk) AS SELECT i, j FROM mvtest_v WITH NO DATA; -- error
! ERROR:  too many column names were specified
! CREATE MATERIALIZED VIEW mvtest_mv_v_3 (ii, jj) AS SELECT i, j FROM mvtest_v WITH NO DATA; -- ok
! CREATE MATERIALIZED VIEW mvtest_mv_v_4 (ii) AS SELECT i, j FROM mvtest_v WITH NO DATA; -- ok
  ALTER TABLE mvtest_v RENAME COLUMN i TO x;
  INSERT INTO mvtest_v values (1, 2);
  CREATE UNIQUE INDEX mvtest_mv_v_ii ON mvtest_mv_v (ii);
  REFRESH MATERIALIZED VIEW mvtest_mv_v;
  UPDATE mvtest_v SET j = 3 WHERE x = 1;
  REFRESH MATERIALIZED VIEW CONCURRENTLY mvtest_mv_v;
+ REFRESH MATERIALIZED VIEW mvtest_mv_v_2;
+ REFRESH MATERIALIZED VIEW mvtest_mv_v_3;
+ REFRESH MATERIALIZED VIEW mvtest_mv_v_4;
  SELECT * FROM mvtest_v;
   x | j
  ---+---
*************** SELECT * FROM mvtest_mv_v;
*** 474,481 ****
    1 |  3
  (1 row)

  DROP TABLE mvtest_v CASCADE;
! NOTICE:  drop cascades to materialized view mvtest_mv_v
  -- make sure that matview rows can be referenced as source rows (bug #9398)
  CREATE TABLE mvtest_v AS SELECT generate_series(1,10) AS a;
  CREATE MATERIALIZED VIEW mvtest_mv_v AS SELECT a FROM mvtest_v WHERE a <= 5;
--- 484,520 ----
    1 |  3
  (1 row)

+ SELECT * FROM mvtest_mv_v_2;
+  ii | j
+ ----+---
+   1 | 3
+ (1 row)
+
+ SELECT * FROM mvtest_mv_v_3;
+  ii | jj
+ ----+----
+   1 |  3
+ (1 row)
+
+ SELECT * FROM mvtest_mv_v_4;
+  ii | j
+ ----+---
+   1 | 3
+ (1 row)
+
  DROP TABLE mvtest_v CASCADE;
! NOTICE:  drop cascades to 4 other objects
! DETAIL:  drop cascades to materialized view mvtest_mv_v
! drop cascades to materialized view mvtest_mv_v_2
! drop cascades to materialized view mvtest_mv_v_3
! drop cascades to materialized view mvtest_mv_v_4
! -- make sure that create WITH NO DATA does not plan the query (bug #13907)
! create materialized view mvtest_error as select 1/0 as x;  -- fail
! ERROR:  division by zero
! create materialized view mvtest_error as select 1/0 as x with no data;
! refresh materialized view mvtest_error;  -- fail here
! ERROR:  division by zero
! drop materialized view mvtest_error;
  -- make sure that matview rows can be referenced as source rows (bug #9398)
  CREATE TABLE mvtest_v AS SELECT generate_series(1,10) AS a;
  CREATE MATERIALIZED VIEW mvtest_mv_v AS SELECT a FROM mvtest_v WHERE a <= 5;
diff --git a/src/test/regress/expected/select_into.out b/src/test/regress/expected/select_into.out
index b577d1b..cee77e7 100644
*** a/src/test/regress/expected/select_into.out
--- b/src/test/regress/expected/select_into.out
*************** DETAIL:  drop cascades to table selinto_
*** 50,55 ****
--- 50,93 ----
  drop cascades to table selinto_schema.tmp2
  drop cascades to table selinto_schema.tmp3
  DROP USER selinto_user;
+ -- Tests for WITH NO DATA and column name consistency
+ CREATE TABLE ctas_base (i int, j int);
+ INSERT INTO ctas_base VALUES (1, 2);
+ CREATE TABLE ctas_nodata (ii, jj, kk) AS SELECT i, j FROM ctas_base; -- Error
+ ERROR:  too many column names were specified
+ CREATE TABLE ctas_nodata (ii, jj, kk) AS SELECT i, j FROM ctas_base WITH NO DATA; -- Error
+ ERROR:  too many column names were specified
+ CREATE TABLE ctas_nodata (ii, jj) AS SELECT i, j FROM ctas_base; -- OK
+ CREATE TABLE ctas_nodata_2 (ii, jj) AS SELECT i, j FROM ctas_base WITH NO DATA; -- OK
+ CREATE TABLE ctas_nodata_3 (ii) AS SELECT i, j FROM ctas_base; -- OK
+ CREATE TABLE ctas_nodata_4 (ii) AS SELECT i, j FROM ctas_base WITH NO DATA; -- OK
+ SELECT * FROM ctas_nodata;
+  ii | jj
+ ----+----
+   1 |  2
+ (1 row)
+
+ SELECT * FROM ctas_nodata_2;
+  ii | jj
+ ----+----
+ (0 rows)
+
+ SELECT * FROM ctas_nodata_3;
+  ii | j
+ ----+---
+   1 | 2
+ (1 row)
+
+ SELECT * FROM ctas_nodata_4;
+  ii | j
+ ----+---
+ (0 rows)
+
+ DROP TABLE ctas_base;
+ DROP TABLE ctas_nodata;
+ DROP TABLE ctas_nodata_2;
+ DROP TABLE ctas_nodata_3;
+ DROP TABLE ctas_nodata_4;
  --
  -- CREATE TABLE AS/SELECT INTO as last command in a SQL function
  -- have been known to cause problems
diff --git a/src/test/regress/sql/matview.sql b/src/test/regress/sql/matview.sql
index 002698a..a108b69 100644
*** a/src/test/regress/sql/matview.sql
--- b/src/test/regress/sql/matview.sql
*************** DROP TABLE mvtest_boxes CASCADE;
*** 176,192 ****

  -- make sure that column names are handled correctly
  CREATE TABLE mvtest_v (i int, j int);
! CREATE MATERIALIZED VIEW mvtest_mv_v (ii) AS SELECT i, j AS jj FROM mvtest_v;
  ALTER TABLE mvtest_v RENAME COLUMN i TO x;
  INSERT INTO mvtest_v values (1, 2);
  CREATE UNIQUE INDEX mvtest_mv_v_ii ON mvtest_mv_v (ii);
  REFRESH MATERIALIZED VIEW mvtest_mv_v;
  UPDATE mvtest_v SET j = 3 WHERE x = 1;
  REFRESH MATERIALIZED VIEW CONCURRENTLY mvtest_mv_v;
  SELECT * FROM mvtest_v;
  SELECT * FROM mvtest_mv_v;
  DROP TABLE mvtest_v CASCADE;

  -- make sure that matview rows can be referenced as source rows (bug #9398)
  CREATE TABLE mvtest_v AS SELECT generate_series(1,10) AS a;
  CREATE MATERIALIZED VIEW mvtest_mv_v AS SELECT a FROM mvtest_v WHERE a <= 5;
--- 176,209 ----

  -- make sure that column names are handled correctly
  CREATE TABLE mvtest_v (i int, j int);
! CREATE MATERIALIZED VIEW mvtest_mv_v (ii, jj, kk) AS SELECT i, j FROM mvtest_v; -- error
! CREATE MATERIALIZED VIEW mvtest_mv_v (ii, jj) AS SELECT i, j FROM mvtest_v; -- ok
! CREATE MATERIALIZED VIEW mvtest_mv_v_2 (ii) AS SELECT i, j FROM mvtest_v; -- ok
! CREATE MATERIALIZED VIEW mvtest_mv_v_3 (ii, jj, kk) AS SELECT i, j FROM mvtest_v WITH NO DATA; -- error
! CREATE MATERIALIZED VIEW mvtest_mv_v_3 (ii, jj) AS SELECT i, j FROM mvtest_v WITH NO DATA; -- ok
! CREATE MATERIALIZED VIEW mvtest_mv_v_4 (ii) AS SELECT i, j FROM mvtest_v WITH NO DATA; -- ok
  ALTER TABLE mvtest_v RENAME COLUMN i TO x;
  INSERT INTO mvtest_v values (1, 2);
  CREATE UNIQUE INDEX mvtest_mv_v_ii ON mvtest_mv_v (ii);
  REFRESH MATERIALIZED VIEW mvtest_mv_v;
  UPDATE mvtest_v SET j = 3 WHERE x = 1;
  REFRESH MATERIALIZED VIEW CONCURRENTLY mvtest_mv_v;
+ REFRESH MATERIALIZED VIEW mvtest_mv_v_2;
+ REFRESH MATERIALIZED VIEW mvtest_mv_v_3;
+ REFRESH MATERIALIZED VIEW mvtest_mv_v_4;
  SELECT * FROM mvtest_v;
  SELECT * FROM mvtest_mv_v;
+ SELECT * FROM mvtest_mv_v_2;
+ SELECT * FROM mvtest_mv_v_3;
+ SELECT * FROM mvtest_mv_v_4;
  DROP TABLE mvtest_v CASCADE;

+ -- make sure that create WITH NO DATA does not plan the query (bug #13907)
+ create materialized view mvtest_error as select 1/0 as x;  -- fail
+ create materialized view mvtest_error as select 1/0 as x with no data;
+ refresh materialized view mvtest_error;  -- fail here
+ drop materialized view mvtest_error;
+
  -- make sure that matview rows can be referenced as source rows (bug #9398)
  CREATE TABLE mvtest_v AS SELECT generate_series(1,10) AS a;
  CREATE MATERIALIZED VIEW mvtest_mv_v AS SELECT a FROM mvtest_v WHERE a <= 5;
diff --git a/src/test/regress/sql/select_into.sql b/src/test/regress/sql/select_into.sql
index e4460ae..632077c 100644
*** a/src/test/regress/sql/select_into.sql
--- b/src/test/regress/sql/select_into.sql
*************** RESET SESSION AUTHORIZATION;
*** 53,58 ****
--- 53,77 ----
  DROP SCHEMA selinto_schema CASCADE;
  DROP USER selinto_user;

+ -- Tests for WITH NO DATA and column name consistency
+ CREATE TABLE ctas_base (i int, j int);
+ INSERT INTO ctas_base VALUES (1, 2);
+ CREATE TABLE ctas_nodata (ii, jj, kk) AS SELECT i, j FROM ctas_base; -- Error
+ CREATE TABLE ctas_nodata (ii, jj, kk) AS SELECT i, j FROM ctas_base WITH NO DATA; -- Error
+ CREATE TABLE ctas_nodata (ii, jj) AS SELECT i, j FROM ctas_base; -- OK
+ CREATE TABLE ctas_nodata_2 (ii, jj) AS SELECT i, j FROM ctas_base WITH NO DATA; -- OK
+ CREATE TABLE ctas_nodata_3 (ii) AS SELECT i, j FROM ctas_base; -- OK
+ CREATE TABLE ctas_nodata_4 (ii) AS SELECT i, j FROM ctas_base WITH NO DATA; -- OK
+ SELECT * FROM ctas_nodata;
+ SELECT * FROM ctas_nodata_2;
+ SELECT * FROM ctas_nodata_3;
+ SELECT * FROM ctas_nodata_4;
+ DROP TABLE ctas_base;
+ DROP TABLE ctas_nodata;
+ DROP TABLE ctas_nodata_2;
+ DROP TABLE ctas_nodata_3;
+ DROP TABLE ctas_nodata_4;
+
  --
  -- CREATE TABLE AS/SELECT INTO as last command in a SQL function
  -- have been known to cause problems