| From: | Sami Imseih <samimseih(at)gmail(dot)com> |
|---|---|
| To: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
| Cc: | Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> |
| Subject: | question about is_valid parameter in set_attnotnull |
| Date: | 2026-07-29 17:59:51 |
| Message-ID: | CAA5RZ0tHnvSrfUy4jWJchjvkL_aJe0hCnZpMsFRdLrSxCne5qQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
(CC'ing Álvaro as the author of a379061a22a8)
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).
My only concern is that the assignment is wrong when is_valid is
false, but nothing currently reads attnullability from the same
relation handle after this point, so it's harmless for now.
Currently, the correct attnullability comes from pg_constraint on the
next relcache rebuild.
Should this be using is_valid to set the attnullability
conditionally, or should the parameter just be removed?
--
Sami Imseih
Amazon Web Services (AWS)
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Alvaro Herrera | 2026-07-29 18:20:12 | Re: question about is_valid parameter in set_attnotnull |
| Previous Message | Greg Burd | 2026-07-29 17:56:41 | Re: [PATCH] Batched clock sweep to reduce cross-socket atomic contention |