Hi,
On 2024-12-05 11:52:01 +1300, David Rowley wrote:
> On Thu, 5 Dec 2024 at 03:51, Andres Freund <andres@anarazel.de> wrote:
> > Possibly stupid idea: Could we instead store the attributes *before* the main
> > TupleDescData, with increasing "distance" for increased attnos? That way we
> > wouldn't need to calculate any variable offsets. Of course the price would be
> > to have some slightly more complicated invocation of pfree(), but that's
> > comparatively rare.
>
> Are you thinking this to make the address calculation cheaper? or so
> that the hacky code that truncates the TupleDesc would work without
> crashing still?
Primarily to make the address calculation cheaper.
> If it's for the latter then the pfree() would be tricky to make work
> still as natts would need to be consulted to find the address to
> pfree.
But is that really a problem? Freeing a tupledesc needs to go through
FreeTupleDesc() (unless a shared one, but that's just one additional place),
and the TupleDesc could just store a pointer/offset to its start.
> > On 2024-12-05 01:42:36 +1300, David Rowley wrote:
> > > Since I'm calculating the base address of the FormData_pg_attribute
> > > array in TupleDesc by looking at natts, when this code changes natts
> > > on the fly, that means calls to TupleDescAttr end up looking in the
> > > wrong place for the required FormData_pg_attribute element.
> >
> > It's possible out-of-core code is doing that too, could we detect this in
> > assert enabled builds?
>
> The assert in TupleDescCompactAttr() which verifies the
> CompactAttribute matches the FormData_pg_attribute did highlight these
> issues.
Cool.
> One way to ensure we purposefully break any code manually adjusting
> natts would be to rename that field. That would mean having to adjust
> all the loops over each attribute in core. There are quite a few:
>
> $ git grep -E "^\s+for.*->natts;" | wc -l
> 147
That doesn't seem worth it...
Greetings,
Andres Freund
