[ Sorry for letting this slip through the cracks ... I think I got distracted by collation bugs :-( ]
Noah Misch <noah@leadboat.com> writes:
> On Sat, Mar 12, 2011 at 12:44:29PM -0500, Tom Lane wrote:
>> Noah Misch <noah@leadboat.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