Re: COPY and default values
От | Tom Lane |
---|---|
Тема | Re: COPY and default values |
Дата | |
Msg-id | 27388.1022534121@sss.pgh.pa.us обсуждение исходный текст |
Ответ на | COPY and default values (Neil Conway <nconway@klamath.dyndns.org>) |
Ответы |
Re: COPY and default values
(Neil Conway <nconway@klamath.dyndns.org>)
|
Список | pgsql-patches |
Neil Conway <nconway@klamath.dyndns.org> writes: > ERROR: copy: line 2, Memory exhausted in AllocSetAlloc(1065320379) > I think I've mismanaged the memory contexts involved somehow, but > I'm not sure what the problem is. Any help would be appreciated... Well, for one thing you're doing + for (i = 0; i < attr_count; i++) + { + if (nulls[i] == 'd' && values[i] == 0) + { + bool isNull; + + values[i] = ExecEvalExprSwitchContext(defaults[i], econtext, + &isNull, NULL); + + /* If it's NULL, return value is meaningless */ + if (isNull) + { + values[i] = 0; + nulls[i] = 'n'; + } + else + nulls[i] = ' '; + + ResetPerTupleExprContext(estate); + } + } which means you reset the memory context containing the default results before those results can ever get used, leaving dangling pointers in values[i]. (Hint: there's a reason why the econtext is called the "per-tuple" context, not "per-column" context. The one reset that's in there now is sufficient.) The nulls = 'd' mechanism is ugly and unnecessary, I think. We were intending to modify COPY's behavior to prohibit missing/extra fields in incoming rows anyway, so there's no reason not to know in advance exactly which columns need to have defaults inserted, and to set up default info for only those columns. I'm also somewhat uncomfortable with the fact that the patch invokes ExecEvalExpr on a NULL pointer if there is a default-less column involved --- perhaps that works at the moment but I don't like it. Should special-case that. The pfree's you've added near line 1021 look rather pointless, seeing as how (a) they're only pfree'ing the topmost node of the default expressions, and (b) the whole context is about to get reset anyhow. There isn't a lot of percentage in any of those end-of-statement pfrees that are there now... I didn't like the aspect of the patch that moves build_column_default into execUtils.c. It's not an executor routine (as evidenced by the fact that you had to add a pile of new inclusions to execUtils.c to make it compile). I'm not sure of the cleanest place for it; maybe someplace in backend/catalog/, though really there's nothing wrong with keeping it in the rewriter. I haven't tried to run the patch, but those were some things I noticed in a quick eyeball review... regards, tom lane
В списке pgsql-patches по дате отправления: