Thanks for looking! > > 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.
At least I could not find any reason for it, but I may have missed something. The function header comment and callers never mention is_valid either; only attnotnull and queue_validation. > Maybe we > don't need thisatt at all and it should also be removed? Right, both the is_valid and thisatt are dead code AFAICT. Here is a patch that cleans it up. -- Sami
v1-0001-Remove-dead-code-in-set_attnotnull.patch
Description: Binary data
