Re: Valgrind - showing memory leaks

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)