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

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).

Thoughts?

Thanks,

Joe
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:13:20 -0000
***************
*** 872,877 ****
--- 872,878 ----
      foreach(lc, exprlist)
      {
          Expr *expr = (Expr *) lfirst(lc);
+         Expr *p = expr;
          ResTarget  *col;

          col = (ResTarget *) lfirst(icols);
***************
*** 885,893 ****
--- 886,898 ----

          result = lappend(result, expr);

+         if (expr != p)
+             pfree(p);
+
          icols = lnext(icols);
          attnos = lnext(attnos);
      }
+     list_free(exprlist);

      return result;
  }
***************
*** 2191,2196 ****
--- 2196,2202 ----
      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
--- 2209,2228 ----
          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:13:21 -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;
  }