Re: question about is_valid parameter in set_attnotnull

Hello Sami,

On 2026-Jul-29, Sami Imseih wrote:

> I was looking at code in tablecmds.c for some other patch and I
> spotted that the is_valid parameter in set_attnotnull(), added in
> a379061a22a8, is never used in the function body. It's not clear
> why it was added, except that it's passed by the caller. The
> CompactAttribute is unconditionally set to ATTNULLABLE_VALID:
>
> ```
> thisatt = TupleDescCompactAttr(RelationGetDescr(rel), attnum - 1);
> thisatt->attnullability = ATTNULLABLE_VALID;
> ```
>
> The only caller that passes is_valid=false is ATAddCheckNNConstraint
> (for NOT VALID constraints).

Hmm, yeah -- as I recall, the intention there is that is_valid would be
false when you want to create an invalid constraint. I suppose it was
used that way during development but we later changed our minds about
the design, made the parameter obsolete, and forgot to set it correctly
or remove it. I would have to study the code to remember in more
detail, but I suspect the right action now is to remove it. Maybe we
don't need thisatt at all and it should also be removed?

But it's time to log off for today ...

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"Hay quien adquiere la mala costumbre de ser infeliz" (M. A. Evans)