Re: Values list-of-targetlists patch for comments (was Re: - Mailing list pgsql-patches

Joe Conway wrote:
> Tom Lane wrote:
>
>> Here's what I've got so far.  I think there's probably more gold to be
>> mined in terms of reducing runtime memory consumption (I don't like the
>> list_free_deep bit, we should use a context), but functionally it seems
>> complete.
>
> I checked out memory usage, and it had regressed to about 1.4 GB (from
> 730 MB as reported yesterday) for 2 million inserts of 2 integers (i.e.
> with the php script I've been using).
>
> I know you're not too happy with the attached approach to solving this,
> but I'm not sure how creating a memory context is going to help. Part of
> the problem is that the various transformXXX functions sometimes return
> freshly palloc'd memory, and sometimes return the pointer they are given.
>
> Anyway, with the attached diff, the 2 million inserts case is back to
> about 730 MB memory use, and speed is pretty much the same as reported
> yesterday (i.e both memory use and performance better than mysql with
> innodb tables).

Of course it also breaks a bunch of regression tests -- I guess that
just points to the fragility of this approach.

This patch retains the memory consumption savings but doesn't break any
regression tests...

Joe
? src/test/regress/sql/insert.sql.new
Index: src/backend/parser/analyze.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/parser/analyze.c,v
retrieving revision 1.341
diff -c -r1.341 analyze.c
*** src/backend/parser/analyze.c    2 Aug 2006 01:59:46 -0000    1.341
--- src/backend/parser/analyze.c    2 Aug 2006 05:48:18 -0000
***************
*** 888,893 ****
--- 888,894 ----
          icols = lnext(icols);
          attnos = lnext(attnos);
      }
+     list_free(exprlist);

      return result;
  }
***************
*** 2191,2196 ****
--- 2192,2198 ----
      for (i = 0; i < sublist_length; i++)
      {
          coltypes[i] = select_common_type(coltype_lists[i], "VALUES");
+         list_free(coltype_lists[i]);
      }

      newExprsLists = NIL;
***************
*** 2203,2216 ****
          foreach(lc2, sublist)
          {
              Node  *col = (Node *) lfirst(lc2);

-             col = coerce_to_common_type(pstate, col, coltypes[i], "VALUES");
-             newsublist = lappend(newsublist, col);
              i++;
          }

          newExprsLists = lappend(newExprsLists, newsublist);
      }

      /*
       * Generate the VALUES RTE
--- 2205,2224 ----
          foreach(lc2, sublist)
          {
              Node  *col = (Node *) lfirst(lc2);
+             Node  *new_col;
+
+             new_col = coerce_to_common_type(pstate, col, coltypes[i], "VALUES");
+             newsublist = lappend(newsublist, new_col);
+             if (new_col != col)
+                 pfree(col);

              i++;
          }

          newExprsLists = lappend(newExprsLists, newsublist);
+         list_free(sublist);
      }
+     list_free(exprsLists);

      /*
       * Generate the VALUES RTE
Index: src/backend/parser/parse_target.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/parser/parse_target.c,v
retrieving revision 1.147
diff -c -r1.147 parse_target.c
*** src/backend/parser/parse_target.c    2 Aug 2006 01:59:47 -0000    1.147
--- src/backend/parser/parse_target.c    2 Aug 2006 05:48:18 -0000
***************
*** 172,177 ****
--- 172,178 ----
      foreach(lc, exprlist)
      {
          Node       *e = (Node *) lfirst(lc);
+         Node       *p = e;

          /*
           * Check for "something.*".  Depending on the complexity of the
***************
*** 188,193 ****
--- 189,195 ----
                  result = list_concat(result,
                                       ExpandColumnRefStar(pstate, cref,
                                                           false));
+                 pfree(e);
                  continue;
              }
          }
***************
*** 203,208 ****
--- 205,211 ----
                  result = list_concat(result,
                                       ExpandIndirectionStar(pstate, ind,
                                                             false));
+                 pfree(e);
                  continue;
              }
          }
***************
*** 210,218 ****
          /*
           * Not "something.*", so transform as a single expression
           */
!         result = lappend(result,
!                          transformExpr(pstate, e));
      }

      return result;
  }
--- 213,224 ----
          /*
           * Not "something.*", so transform as a single expression
           */
!         p = transformExpr(pstate, e);
!         result = lappend(result, p);
!         if (e != p)
!             pfree(e);
      }
+     list_free(exprlist);

      return result;
  }