Re: BUG #17633: Define rule on views which do insert to another relation trigger cache lookup failed error.

I wrote:
> I think the basic problem is that the two calls of rewriteValuesRTE
> are really dealing with fundamentally different cases, and we should
> probably not have tried to make the same function do both.  I'm going
> to try splitting it into two functions, one for the force_nulls case
> and one for the !force_nulls case.

As attached.  This seems *way* cleaner to me, even if it means we need
two copies of the loops-over-VALUES-list-entries.  I didn't write a
regression test yet, but this fixes the submitted bug and passes
check-world.

            regards, tom lane

diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index d02fd83c0a..a6dd99cc98 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -79,8 +79,9 @@ static TargetEntry *process_matched_tle(TargetEntry *src_tle,
 static Node *get_assignment_input(Node *node);
 static Bitmapset *findDefaultOnlyColumns(RangeTblEntry *rte);
 static bool rewriteValuesRTE(Query *parsetree, RangeTblEntry *rte, int rti,
-                             Relation target_relation, bool force_nulls,
+                             Relation target_relation,
                              Bitmapset *unused_cols);
+static void rewriteValuesRTEToNulls(Query *parsetree, RangeTblEntry *rte);
 static void markQueryForLocking(Query *qry, Node *jtnode,
                                 LockClauseStrength strength, LockWaitPolicy waitPolicy,
                                 bool pushedDown);
@@ -1370,18 +1371,7 @@ findDefaultOnlyColumns(RangeTblEntry *rte)
  * all DEFAULT items are replaced, and if the target relation doesn't have a
  * default, the value is explicitly set to NULL.
  *
- * Additionally, if force_nulls is true, the target relation's defaults are
- * ignored and all DEFAULT items in the VALUES list are explicitly set to
- * NULL, regardless of the target relation's type.  This is used for the
- * product queries generated by DO ALSO rules attached to an auto-updatable
- * view, for which we will have already called this function with force_nulls
- * false.  For these product queries, we must then force any remaining DEFAULT
- * items to NULL to provide concrete values for the rule actions.
- * Essentially, this is a mix of the 2 cases above --- the original query is
- * an insert into an auto-updatable view, and the product queries are inserts
- * into a rule-updatable view.
- *
- * Finally, if a DEFAULT item is found in a column mentioned in unused_cols,
+ * Also, if a DEFAULT item is found in a column mentioned in unused_cols,
  * it is explicitly set to NULL.  This happens for columns in the VALUES RTE
  * whose corresponding targetlist entries have already been replaced with the
  * relation's default expressions, so that any values in those columns of the
@@ -1404,7 +1394,7 @@ findDefaultOnlyColumns(RangeTblEntry *rte)
  */
 static bool
 rewriteValuesRTE(Query *parsetree, RangeTblEntry *rte, int rti,
-                 Relation target_relation, bool force_nulls,
+                 Relation target_relation,
                  Bitmapset *unused_cols)
 {
     List       *newValues;
@@ -1414,15 +1404,14 @@ rewriteValuesRTE(Query *parsetree, RangeTblEntry *rte, int rti,
     int            numattrs;
     int           *attrnos;

+    Assert(rte->rtekind == RTE_VALUES);
+
     /*
      * Rebuilding all the lists is a pretty expensive proposition in a big
      * VALUES list, and it's a waste of time if there aren't any DEFAULT
      * placeholders.  So first scan to see if there are any.
-     *
-     * We skip this check if force_nulls is true, because we know that there
-     * are DEFAULT items present in that case.
      */
-    if (!force_nulls && !searchForDefault(rte))
+    if (!searchForDefault(rte))
         return true;            /* nothing to do */

     /*
@@ -1456,12 +1445,10 @@ rewriteValuesRTE(Query *parsetree, RangeTblEntry *rte, int rti,
     /*
      * Check if the target relation is an auto-updatable view, in which case
      * unresolved defaults will be left untouched rather than being set to
-     * NULL.  If force_nulls is true, we always set DEFAULT items to NULL, so
-     * skip this check in that case --- it isn't an auto-updatable view.
+     * NULL.
      */
     isAutoUpdatableView = false;
-    if (!force_nulls &&
-        target_relation->rd_rel->relkind == RELKIND_VIEW &&
+    if (target_relation->rd_rel->relkind == RELKIND_VIEW &&
         !view_has_instead_trigger(target_relation, CMD_INSERT))
     {
         List       *locks;
@@ -1535,9 +1522,10 @@ rewriteValuesRTE(Query *parsetree, RangeTblEntry *rte, int rti,

                 if (attrno == 0)
                     elog(ERROR, "cannot set value in column %d to DEFAULT", i);
+                Assert(attrno > 0 && attrno <= target_relation->rd_att->natts);
                 att_tup = TupleDescAttr(target_relation->rd_att, attrno - 1);

-                if (!force_nulls && !att_tup->attisdropped)
+                if (!att_tup->attisdropped)
                     new_expr = build_column_default(target_relation, attrno);
                 else
                     new_expr = NULL;    /* force a NULL if dropped */
@@ -1587,6 +1575,50 @@ rewriteValuesRTE(Query *parsetree, RangeTblEntry *rte, int rti,
     return allReplaced;
 }

+/*
+ * Mop up any remaining DEFAULT items in the given VALUES RTE by
+ * replacing them with NULL constants.
+ *
+ * This is used for the product queries generated by DO ALSO rules attached to
+ * an auto-updatable view.  The action can't depend on the "target relation"
+ * since the product query might not have one (it needn't be an INSERT).
+ * Essentially, such queries are treated as being attached to a rule-updatable
+ * view.
+ */
+static void
+rewriteValuesRTEToNulls(Query *parsetree, RangeTblEntry *rte)
+{
+    List       *newValues;
+    ListCell   *lc;
+
+    Assert(rte->rtekind == RTE_VALUES);
+    newValues = NIL;
+    foreach(lc, rte->values_lists)
+    {
+        List       *sublist = (List *) lfirst(lc);
+        List       *newList = NIL;
+        ListCell   *lc2;
+
+        foreach(lc2, sublist)
+        {
+            Node       *col = (Node *) lfirst(lc2);
+
+            if (IsA(col, SetToDefault))
+            {
+                SetToDefault *def = (SetToDefault *) col;
+
+                newList = lappend(newList, makeNullConst(def->typeId,
+                                                         def->typeMod,
+                                                         def->collation));
+            }
+            else
+                newList = lappend(newList, col);
+        }
+        newValues = lappend(newValues, newList);
+    }
+    rte->values_lists = newValues;
+}
+

 /*
  * Record in target_rte->extraUpdatedCols the indexes of any generated columns
@@ -3746,7 +3778,7 @@ RewriteQuery(Query *parsetree, List *rewrite_events)
                                                             &unused_values_attrnos);
                 /* ... and the VALUES expression lists */
                 if (!rewriteValuesRTE(parsetree, values_rte, values_rte_index,
-                                      rt_entry_relation, false,
+                                      rt_entry_relation,
                                       unused_values_attrnos))
                     defaults_remaining = true;
             }
@@ -3860,10 +3892,7 @@ RewriteQuery(Query *parsetree, List *rewrite_events)
                 RangeTblEntry *values_rte = rt_fetch(values_rte_index,
                                                      pt->rtable);

-                rewriteValuesRTE(pt, values_rte, values_rte_index,
-                                 rt_entry_relation,
-                                 true,    /* Force remaining defaults to NULL */
-                                 NULL);
+                rewriteValuesRTEToNulls(pt, values_rte);
             }
         }