Re: Materialized view assertion failure in HEAD

I wrote:
> BTW, is there a really solid reason why a matview couldn't be allowed to
> have OIDs on demand, and thereby dodge this whole problem?  I'm thinking
> that the analogy to regular views not having OIDs is not a very good
> argument, because certainly matview rows are going to need all the other
> system columns.

Some experimentation says that actually it works just fine to create
matviews with OIDs; the only part of the backend that has a problem with
that is REFRESH MATERIALIZED VIEW, which is easily fixed as per
attached.

If we go in this direction it would probably also be a good idea to
revert the hack in psql/describe.c to prevent it from printing Has OIDs
for matviews.

Also, the fact that Joachim saw this problem in a dump/reload implies
that pg_dump is failing to preserve the state of relhasoids for
matviews, because tvmm hasn't got OIDs in the regression database, but
his stack trace says it did after dump and reload.  I haven't looked
into how come, but whichever way we jump as far as the backend behavior,
pg_dump is clearly confused.

Anyway, I think it makes more sense to go in the direction of making
the case work than to introduce messy kluges to prevent matviews from
having OIDs.

Also ... while I was nosing around I noticed this bit in InitPlan():

      /*
+      * Unless we are creating a view or are creating a materialized view WITH
+      * NO DATA, ensure that all referenced relations are scannable.
+      */
+     if ((eflags & EXEC_FLAG_WITH_NO_DATA) == 0)
+         ExecCheckRelationsScannable(rangeTable);

ISTM that suppressing this check during a matview refresh is rather
broken, because then it won't complain if the matview reads some other
matview that's in a nonscannable state.  Is that really the behavior
we want?  (Scanning the rangetable is probably wrong for this anyway
--- it'd be better to put the responsibility into the table-scan-node
initialization functions.)

            regards, tom lane

diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c
index e20fedaeaf77772a47b34f423ddeb062b429bf1f..dbcbf332539b00516afbb28be56857a5f9c2402f 100644
*** a/src/backend/commands/matview.c
--- b/src/backend/commands/matview.c
*************** typedef struct
*** 44,55 ****
      BulkInsertState bistate;    /* bulk insert state */
  } DR_transientrel;

  static void transientrel_startup(DestReceiver *self, int operation, TupleDesc typeinfo);
  static void transientrel_receive(TupleTableSlot *slot, DestReceiver *self);
  static void transientrel_shutdown(DestReceiver *self);
  static void transientrel_destroy(DestReceiver *self);
- static void refresh_matview_datafill(DestReceiver *dest, Query *query,
-                                      const char *queryString);

  /*
   * SetRelationIsScannable
--- 44,55 ----
      BulkInsertState bistate;    /* bulk insert state */
  } DR_transientrel;

+ static void refresh_matview_datafill(DestReceiver *dest, bool hasoids,
+                                      Query *query, const char *queryString);
  static void transientrel_startup(DestReceiver *self, int operation, TupleDesc typeinfo);
  static void transientrel_receive(TupleTableSlot *slot, DestReceiver *self);
  static void transientrel_shutdown(DestReceiver *self);
  static void transientrel_destroy(DestReceiver *self);

  /*
   * SetRelationIsScannable
*************** ExecRefreshMatView(RefreshMatViewStmt *s
*** 138,145 ****
       */
      Assert(!IsSystemRelation(matviewRel));

-     Assert(!matviewRel->rd_rel->relhasoids);
-
      /*
       * Check that everything is correct for a refresh. Problems at this point
       * are internal errors, so elog is sufficient.
--- 138,143 ----
*************** ExecRefreshMatView(RefreshMatViewStmt *s
*** 185,198 ****

      tableSpace = matviewRel->rd_rel->reltablespace;

-     heap_close(matviewRel, NoLock);
-
      /* Create the transient table that will receive the regenerated data. */
      OIDNewHeap = make_new_heap(matviewOid, tableSpace);
      dest = CreateTransientRelDestReceiver(OIDNewHeap);

      if (!stmt->skipData)
!         refresh_matview_datafill(dest, dataQuery, queryString);

      /*
       * Swap the physical files of the target and transient tables, then
--- 183,197 ----

      tableSpace = matviewRel->rd_rel->reltablespace;

      /* Create the transient table that will receive the regenerated data. */
      OIDNewHeap = make_new_heap(matviewOid, tableSpace);
      dest = CreateTransientRelDestReceiver(OIDNewHeap);

      if (!stmt->skipData)
!         refresh_matview_datafill(dest, matviewRel->rd_rel->relhasoids,
!                                  dataQuery, queryString);
!
!     heap_close(matviewRel, NoLock);

      /*
       * Swap the physical files of the target and transient tables, then
*************** ExecRefreshMatView(RefreshMatViewStmt *s
*** 208,215 ****
   * refresh_matview_datafill
   */
  static void
! refresh_matview_datafill(DestReceiver *dest, Query *query,
!                          const char *queryString)
  {
      List       *rewritten;
      PlannedStmt *plan;
--- 207,214 ----
   * refresh_matview_datafill
   */
  static void
! refresh_matview_datafill(DestReceiver *dest, bool hasoids,
!                          Query *query, const char *queryString)
  {
      List       *rewritten;
      PlannedStmt *plan;
*************** refresh_matview_datafill(DestReceiver *d
*** 217,222 ****
--- 216,222 ----
      List       *rtable;
      RangeTblEntry    *initial_rte;
      RangeTblEntry    *second_rte;
+     int            eflags;

      rewritten = QueryRewrite((Query *) copyObject(query));

*************** refresh_matview_datafill(DestReceiver *d
*** 265,272 ****
                                  GetActiveSnapshot(), InvalidSnapshot,
                                  dest, NULL, 0);

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

      /* run the plan */
      ExecutorRun(queryDesc, ForwardScanDirection, 0L);
--- 265,281 ----
                                  GetActiveSnapshot(), InvalidSnapshot,
                                  dest, NULL, 0);

+     /*
+      * Tell the executor whether to produce OIDs or not (we need this hack
+      * because the executor doesn't see the matview as a target relation).
+      */
+     if (hasoids)
+         eflags = EXEC_FLAG_WITH_OIDS;
+     else
+         eflags = EXEC_FLAG_WITHOUT_OIDS;
+
      /* call ExecutorStart to prepare the plan for execution */
!     ExecutorStart(queryDesc, eflags);

      /* run the plan */
      ExecutorRun(queryDesc, ForwardScanDirection, 0L);
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 4ce831a433523711a81f95588c839e3f4018e6ec..57235ef792bcadec111e2193a29d8a244def5c33 100644
*** a/src/bin/psql/describe.c
--- b/src/bin/psql/describe.c
*************** describeOneTableDetails(const char *sche
*** 2266,2273 ****
              printTableAddFooter(&cont, buf.data);
          }

!         /* OIDs, if verbose and not a materialized view */
!         if (verbose && tableinfo.relkind != 'm')
          {
              const char *s = _("Has OIDs");

--- 2266,2273 ----
              printTableAddFooter(&cont, buf.data);
          }

!         /* OIDs, if verbose */
!         if (verbose)
          {
              const char *s = _("Has OIDs");