Hi, On 2024-12-05 11:52:01 +1300, David Rowley wrote: > On Thu, 5 Dec 2024 at 03:51, Andres Freund <and...@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
