On 2025-May-08, Tom Lane wrote: > Uh ... yeah it is, down at the bottom of the function: > > /* Install array only after it's fully valid */ > relation->rd_att->constr->check = check; > relation->rd_att->constr->num_check = found; > > So it seems like valgrind is wrong here, or else we're leaking the > whole rd_att structure later on somehow.
Well, the problem is that if num_check is zero, FreeTupleDesc() doesn't free ->check. > In any case, you're right that asking for a zero-size chunk is pretty > pointless. I'd support doing > > + if (ncheck > 0) > + check = (ConstrCheck *) > + MemoryContextAllocZero(CacheMemoryContext, > + ncheck * > sizeof(ConstrCheck)); > + else > + check = NULL; > > but I think we have to make sure it's null if we don't palloc it. Done that way, thanks. > > On the other hand, the bug I was thinking about, is that if the table > > has an invalid not-null constraint, we leak during detoasting in > > extractNotNullColumn(). [...] But it's no longer so obvious that > > extractNotNullColumn is okay to leak those few bytes. > > Given your description it still sounds fine to me. Cool, I left it alone. -- Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/ “Cuando no hay humildad las personas se degradan” (A. Christie)
