Hi, > This patch set applies the explicit catalog representation of not-null > constraints introduced by b0e96f3119 for table constraints also to > domain not-null constraints.
Interestingly enough according to the documentation this syntax is already supported [1][2], but the actual query will fail on `master`: ``` =# create domain connotnull integer; CREATE DOMAIN =# alter domain connotnull add not null value; ERROR: unrecognized constraint subtype: 1 ``` I wonder if we should reflect this limitation in the documentation and/or show better error messages. This could be quite surprising to the user. However if we change the documentation on the `master` branch this patch will have to change it back. I was curious about the semantic difference between `SET NOT NULL` and `ADD NOT NULL value`. When I wanted to figure this out I discovered something that seems to be a bug: ``` =# create domain connotnull1 integer; =# create domain connotnull2 integer; =# alter domain connotnull1 add not null value; =# alter domain connotnull2 set not null; =# \dD ERROR: unexpected null value in cached tuple for catalog pg_constraint column conkey ``` Also it turned out that I can do both: `SET NOT NULL` and `ADD NOT NULL value` for the same domain. Is it an intended behavior? We should either forbid it or cover this case with a test. NOT VALID is not supported: ``` =# alter domain connotnull add not null value not valid; ERROR: NOT NULL constraints cannot be marked NOT VALID ``` ... and this is correct: "NOT VALID is only accepted for CHECK constraints" [1]. This code path however doesn't seem to be test-covered even on `master`. While on it, I suggest fixing this. [1]: https://www.postgresql.org/docs/current/sql-alterdomain.html [2]: https://www.postgresql.org/docs/current/sql-createdomain.html -- Best regards, Aleksander Alekseev
