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)
