Re: TupleDescCopy doesn't clear atthasdef, attnotnull, attidentity - Mailing list pgsql-hackers

From Thomas Munro
Subject Re: TupleDescCopy doesn't clear atthasdef, attnotnull, attidentity
Date
Msg-id CAEepm=1kbLW+VG-z9KUCf+o=bwau1T5hJc8sTMMnmpaSeJ6+HA@mail.gmail.com
Whole thread Raw
In response to Re: TupleDescCopy doesn't clear atthasdef, attnotnull, attidentity  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: TupleDescCopy doesn't clear atthasdef, attnotnull, attidentity
Re: TupleDescCopy doesn't clear atthasdef, attnotnull, attidentity
List pgsql-hackers
On Thu, Jan 4, 2018 at 12:01 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Thomas Munro <thomas.munro@enterprisedb.com> writes:
>> Andrew Gierth complained off-list that TupleDescCopy() doesn't clear
>> atthasdef.  Yeah, that's an oversight.  The function is new in commit
>> cc5f81366c36 and was written by me to support "flat" (pointer-free)
>> tuple descriptors for use in DSM.  Following the example of
>> CreateTupleDescCopy() I think it should also clear attnotnull and
>> attidentity.  Please see attached.
>
> I've pushed this with some editorialization.  I added some comments, and
> noting that you have TupleDescCopy() copying the entire attribute array
> in one memcpy, I propagated that same approach into CreateTupleDescCopy
> and CreateTupleDescCopyConstr.  This should make things a bit faster
> since memcpy can spend more time in its maximum-stride loop.

Thanks.

> The reason I note this explicitly is that I don't find it to be
> entirely safe.  If ATTRIBUTE_FIXED_PART_SIZE were less than
> sizeof(FormData_pg_attribute) due to alignment padding at the end of
> the struct, I think we would get some valgrind complaints about copying
> uninitialized data, since there are code paths in which only the first
> ATTRIBUTE_FIXED_PART_SIZE bytes of each array entry get filled in.
> Now, currently I don't think there's any padding there anyway on any
> platform we support.  But if we're going to do things like this, I think
> that we ought to explicitly make ATTRIBUTE_FIXED_PART_SIZE the same as
> sizeof(FormData_pg_attribute).  Hence, I propose the attached follow-on
> patch.

That seems correct.

If there is any system where sizeof(FormData_pg_attribute) !=
(offsetof(FormData_pg_attribute,attcollation) + sizeof(Oid)), won't
load_relcache_init_file() get upset?  Oh, I see it would just go to
read_failed and then "do it the hard way".

+1

-- 
Thomas Munro
http://www.enterprisedb.com


pgsql-hackers by date:

Previous
From: Stephen Frost
Date:
Subject: Re: CFM for January commitfest?
Next
From: Thomas Munro
Date:
Subject: Re: TupleDescCopy doesn't clear atthasdef, attnotnull, attidentity