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?
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.
> 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. It's just it wasn't that obvious what the cause of the issue
was from that failing assert. I expect there would be some breaking of
extensions by removing the attrs array anyway. 65b71dec2d fixed up
some cases where TupleDescAttr() wasn't being used, so anything out
there that's doing something similar to what that commit fixed would
fail to compile.
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
David
