Re: Catalog domain not-null constraints

From: Aleksander Alekseev <aleksander(at)timescale(dot)com>
To: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Cc: Peter Eisentraut <peter(at)eisentraut(dot)org>
Subject: Re: Catalog domain not-null constraints
Date: 2023-11-23 13:13:30
Message-ID: CAJ7c6TMXq-s6oaG-PV7h_He18WqeigehD-VF9Ga4UiQC-Cz06Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Heikki Linnakangas 2023-11-23 13:26:23 Re: PoC: prefetching index leaf pages (for inserts)
Previous Message Alexander Korotkov 2023-11-23 12:42:49 Table AM Interface Enhancements