From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Noah Misch <noah(at)leadboat(dot)com> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: memory-related bugs |
Date: | 2011-09-06 19:00:42 |
Message-ID: | 18540.1315335642@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
[ Sorry for letting this slip through the cracks ... I think I got
distracted by collation bugs :-( ]
Noah Misch <noah(at)leadboat(dot)com> writes:
> On Sat, Mar 12, 2011 at 12:44:29PM -0500, Tom Lane wrote:
>> Noah Misch <noah(at)leadboat(dot)com> writes:
>>> A suitably-instrumented run of "make installcheck-world" under valgrind turned
>>> up a handful of memory-related bugs:
>> Hmm, interesting work, but I don't think I believe in the necessity for
>> this kluge:
>>
> + else if (attributeName != &(att->attname))
> + namestrcpy(&(att->attname), attributeName);
I'm still of the opinion that there's no real need to avoid memcpy with
identical source and destination, so I didn't apply this first patch.
>> I wonder whether we should instead fix this by copying the correct tuple
>> length.
> Seems like a step in the wrong direction. We only use typlen and typbyval
> beyond the immediate context.
Well, the whole point of exposing the pg_type tuple is to avoid making
assumptions about what parts of it the type-specific analyze routine
will wish to look at. If anything we ought to move in the direction of
allowing the non-fixed fields to be accessible too. I didn't do that,
since it would imply an ABI change (to add a HeapTuple field to
VacAttrStats) and would therefore not be back-patchable. But I did
change the code to use SearchSysCacheCopy, which fixes this bug and
is readily extensible if we do decide to add such a field later.
I applied your third patch (for SJIS2004 conversion) as-is.
Thanks for the report and testing!
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | Bruce Momjian | 2011-09-06 19:14:53 | Re: timezone GUC |
Previous Message | Josh Berkus | 2011-09-06 18:41:01 | Re: Alpha 1 for 9.2 |