Обсуждение: Catalog domain not-null constraints
This patch set applies the explicit catalog representation of not-null constraints introduced by b0e96f3119 for table constraints also to domain not-null constraints. Since there is no inheritance or primary keys etc., this is much simpler and just applies the existing infrastructure to domains as well. As a result, domain not-null constraints now appear in the information schema correctly. Another effect is that you can now use the ALTER DOMAIN ... ADD/DROP CONSTRAINT syntax for not-null constraints as well. This makes everything consistent overall. For the most part, I structured the code so that there are now separate sibling subroutines for domain check constraints and domain not-null constraints. This seemed to work out best, but one could also consider other ways to refactor this.
Вложения
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
On 2023-Nov-23, Aleksander Alekseev wrote: > 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 > ``` Hah, nice ... this only fails in this way on master, though, as a side-effect of previous NOT NULL work during this cycle. So if we take Peter's patch, we don't need to worry about it. In 16 it behaves properly, with a normal syntax error. > ``` > =# 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 > ``` This is also a master-only problem, as "add not null" is rejected in 16 with a syntax error (and obviously \dD doesn't fail). > NOT VALID is not supported: > > ``` > =# alter domain connotnull add not null value not valid; > ERROR: NOT NULL constraints cannot be marked NOT VALID > ``` Yeah, it'll take more work to let NOT NULL constraints be marked NOT VALID, both on domains and on tables. It'll be a good feature for sure. -- Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/ "La victoria es para quien se atreve a estar solo"
On 2023-Nov-23, Peter Eisentraut wrote: > This patch set applies the explicit catalog representation of not-null > constraints introduced by b0e96f3119 for table constraints also to domain > not-null constraints. I like the idea of having domain not-null constraints appear in pg_constraint. > Since there is no inheritance or primary keys etc., this is much simpler and > just applies the existing infrastructure to domains as well. If you create a table with column of domain that has a NOT NULL constraint, what happens? I mean, is the table column marked attnotnull, and how does it behave? Is there a separate pg_constraint row for the constraint in the table? What happens if you do ALTER TABLE ... DROP NOT NULL for that column? -- Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/ "Por suerte hoy explotó el califont porque si no me habría muerto de aburrido" (Papelucho)
On 23.11.23 17:38, Alvaro Herrera wrote: > If you create a table with column of domain that has a NOT NULL > constraint, what happens? I mean, is the table column marked > attnotnull, and how does it behave? No, the domain does not affect the catalog entry for the column. This is the same way it behaves now. > Is there a separate pg_constraint > row for the constraint in the table? What happens if you do > ALTER TABLE ... DROP NOT NULL for that column? Those are separate. After dropping the NOT NULL for a column, null values for the column could still be rejected by a domain. (This is the same way CHECK constraints work.)
On 23.11.23 14:13, Aleksander Alekseev wrote: > =# 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 Yeah, for domain not-null constraints pg_constraint.conkey is indeed null. Should we put something in there? Attached is an updated patch that avoids the error by taking a separate code path for domain constraints in ruleutils.c. But maybe there is another way to arrange this.
Вложения
On Wed, 29 Nov 2023 at 01:14, Peter Eisentraut <peter@eisentraut.org> wrote:
>
> On 23.11.23 14:13, Aleksander Alekseev wrote:
> > =# 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
>
> Yeah, for domain not-null constraints pg_constraint.conkey is indeed
> null. Should we put something in there?
>
> Attached is an updated patch that avoids the error by taking a separate
> code path for domain constraints in ruleutils.c. But maybe there is
> another way to arrange this.
One of the test has failed in CFBot at [1] with:
diff -U3 /tmp/cirrus-ci-build/src/test/regress/expected/domain.out
/tmp/cirrus-ci-build/src/test/regress/results/domain.out
--- /tmp/cirrus-ci-build/src/test/regress/expected/domain.out
2024-01-14 15:40:01.793434601 +0000
+++ /tmp/cirrus-ci-build/src/test/regress/results/domain.out
2024-01-14 15:42:23.013332625 +0000
@@ -1271,11 +1271,4 @@
FROM information_schema.domain_constraints
WHERE domain_name IN ('con', 'dom', 'pos_int', 'things'))
ORDER BY constraint_name;
- constraint_catalog | constraint_schema | constraint_name | check_clause
---------------------+-------------------+------------------+-------------------
- regression | public | con_check | (VALUE > 0)
- regression | public | meow | (VALUE < 11)
- regression | public | pos_int_check | (VALUE > 0)
- regression | public | pos_int_not_null | VALUE IS NOT NULL
-(4 rows)
-
+ERROR: could not open relation with OID 36379
[1] - https://cirrus-ci.com/task/4536440638406656
[2] - https://api.cirrus-ci.com/v1/artifact/task/4536440638406656/log/src/test/regress/regression.diffs
Regards,
Vignesh
On 17.01.24 13:15, vignesh C wrote:
> One of the test has failed in CFBot at [1] with:
> diff -U3 /tmp/cirrus-ci-build/src/test/regress/expected/domain.out
> /tmp/cirrus-ci-build/src/test/regress/results/domain.out
> --- /tmp/cirrus-ci-build/src/test/regress/expected/domain.out
> 2024-01-14 15:40:01.793434601 +0000
> +++ /tmp/cirrus-ci-build/src/test/regress/results/domain.out
> 2024-01-14 15:42:23.013332625 +0000
> @@ -1271,11 +1271,4 @@
> FROM information_schema.domain_constraints
> WHERE domain_name IN ('con', 'dom', 'pos_int', 'things'))
> ORDER BY constraint_name;
> - constraint_catalog | constraint_schema | constraint_name | check_clause
> ---------------------+-------------------+------------------+-------------------
> - regression | public | con_check | (VALUE > 0)
> - regression | public | meow | (VALUE < 11)
> - regression | public | pos_int_check | (VALUE > 0)
> - regression | public | pos_int_not_null | VALUE IS NOT NULL
> -(4 rows)
> -
> +ERROR: could not open relation with OID 36379
>
> [1] - https://cirrus-ci.com/task/4536440638406656
> [2] - https://api.cirrus-ci.com/v1/artifact/task/4536440638406656/log/src/test/regress/regression.diffs
Interesting. I couldn't reproduce this locally, even across different
operating systems. The cfbot failures appear to be sporadic, but also
happening across multiple systems, so it's clearly not just a local
environment failure. Can anyone else perhaps reproduce this locally?
On 18.01.24 07:53, Peter Eisentraut wrote:
> On 17.01.24 13:15, vignesh C wrote:
>> One of the test has failed in CFBot at [1] with:
>> diff -U3 /tmp/cirrus-ci-build/src/test/regress/expected/domain.out
>> /tmp/cirrus-ci-build/src/test/regress/results/domain.out
>> --- /tmp/cirrus-ci-build/src/test/regress/expected/domain.out
>> 2024-01-14 15:40:01.793434601 +0000
>> +++ /tmp/cirrus-ci-build/src/test/regress/results/domain.out
>> 2024-01-14 15:42:23.013332625 +0000
>> @@ -1271,11 +1271,4 @@
>> FROM information_schema.domain_constraints
>> WHERE domain_name IN ('con', 'dom', 'pos_int', 'things'))
>> ORDER BY constraint_name;
>> - constraint_catalog | constraint_schema | constraint_name |
>> check_clause
>> ---------------------+-------------------+------------------+-------------------
>> - regression | public | con_check | (VALUE > 0)
>> - regression | public | meow | (VALUE <
>> 11)
>> - regression | public | pos_int_check | (VALUE > 0)
>> - regression | public | pos_int_not_null | VALUE IS
>> NOT NULL
>> -(4 rows)
>> -
>> +ERROR: could not open relation with OID 36379
>>
>> [1] - https://cirrus-ci.com/task/4536440638406656
>> [2] -
>> https://api.cirrus-ci.com/v1/artifact/task/4536440638406656/log/src/test/regress/regression.diffs
>
> Interesting. I couldn't reproduce this locally, even across different
> operating systems. The cfbot failures appear to be sporadic, but also
> happening across multiple systems, so it's clearly not just a local
> environment failure. Can anyone else perhaps reproduce this locally?
This patch set needed a rebase, so here it is.
About the sporadic test failure above, I think that is an existing issue
that is just now exposed through some test timing changes. The
pg_get_expr() function used in information_schema.check_constraints has
no locking against concurrent drops of tables. I think in this
particular case, the tests "domain" and "alter_table" are prone to this
conflict. If I move "domain" to a separate test group, the issue goes
away. I'll start a separate discussion about this issue.
Вложения
On Wed, Feb 7, 2024 at 4:11 PM Peter Eisentraut <peter@eisentraut.org> wrote:
>
> >
> > Interesting. I couldn't reproduce this locally, even across different
> > operating systems. The cfbot failures appear to be sporadic, but also
> > happening across multiple systems, so it's clearly not just a local
> > environment failure. Can anyone else perhaps reproduce this locally?
>
> This patch set needed a rebase, so here it is.
>
do you think
add following
ALTER DOMAIN <replaceable class="parameter">name</replaceable> ADD NOT
NULL VALUE
to doc/src/sgml/ref/alter_domain.sgml synopsis makes sense?
otherwise it would be hard to find out this command, i think.
I think I found a bug.
connotnull already set to not null.
every execution of `alter domain connotnull add not null value ;`
would concatenate 'NOT NULL VALUE' for the "Check" column,
That means changes in the function pg_get_constraintdef_worker are not
100% correct.
see below demo:
src8=# \dD+
List of domains
Schema | Name | Type | Collation | Nullable | Default |
Check | Access privileges | Description
--------+------------+---------+-----------+----------+---------+----------------+-------------------+-------------
public | connotnull | integer | | | | NOT
NULL VALUE | |
public | nnint | integer | | not null | | NOT
NULL VALUE | |
(2 rows)
src8=# alter domain connotnull add not null value ;
ALTER DOMAIN
src8=# \dD+
List of domains
Schema | Name | Type | Collation | Nullable | Default |
Check | Access privileges | Descript
ion
--------+------------+---------+-----------+----------+---------+-------------------------------+-------------------+---------
----
public | connotnull | integer | | not null | | NOT
NULL VALUE NOT NULL VALUE | |
public | nnint | integer | | not null | | NOT
NULL VALUE | |
(2 rows)
src8=# alter domain connotnull add not null value ;
ALTER DOMAIN
src8=# \dD+
List of domains
Schema | Name | Type | Collation | Nullable | Default |
Check | Access privil
eges | Description
--------+------------+---------+-----------+----------+---------+----------------------------------------------+--------------
-----+-------------
public | connotnull | integer | | not null | | NOT
NULL VALUE NOT NULL VALUE NOT NULL VALUE |
|
public | nnint | integer | | not null | | NOT
NULL VALUE |
|
(2 rows)
On 08.02.24 13:17, jian he wrote: > I think I found a bug. > connotnull already set to not null. > every execution of `alter domain connotnull add not null value ;` > would concatenate 'NOT NULL VALUE' for the "Check" column, I would have expected that. Each invocation adds a new constraint. But I see that table constraints do not work that way. A command like ALTER TABLE t1 ADD NOT NULL c1 does nothing if the column already has a NOT NULL constraint. I'm not sure this is correct. At least it's not documented. We should probably make the domains feature work the same way, but I would like to understand why it works that way first.
Peter Eisentraut <peter@eisentraut.org> writes:
> But I see that table constraints do not work that way. A command like
> ALTER TABLE t1 ADD NOT NULL c1 does nothing if the column already has a
> NOT NULL constraint. I'm not sure this is correct. At least it's not
> documented. We should probably make the domains feature work the same
> way, but I would like to understand why it works that way first.
That's probably a hangover from when the underlying state was just
a boolean (attnotnull). Still, I'm a little hesitant to change the
behavior. I do agree that named constraints need to "stack", so
that you'd have to remove each one before not-nullness stops being
enforced. Less sure about unnamed properties.
regards, tom lane
On 2024-Feb-11, Peter Eisentraut wrote: > But I see that table constraints do not work that way. A command like ALTER > TABLE t1 ADD NOT NULL c1 does nothing if the column already has a NOT NULL > constraint. I'm not sure this is correct. At least it's not documented. > We should probably make the domains feature work the same way, but I would > like to understand why it works that way first. It's an intentional design decision actually; I had it creating multiple constraints at first, but it had some ugly problems, so I made it behave this way (which was no small amount of changes). I think the first time I posted an implementation that worked this way was here https://postgr.es/m/20230315224440.cz3brr6323fcrxs6@alvherre.pgsql and then we debated it again later, starting at the bottom of https://www.postgresql.org/message-id/flat/CAEZATCUA_iPo5kqUun4myghoZtgqbY3jx62%3DGwcYKRMmxFUq_g%40mail.gmail.com#482db1d21bcf8a4c3ef4fbee609810f4 A few messages later, I quoted the SQL standard for DROP NOT NULL, which is pretty clear that if you run that command, then the column becomes possibly nullable, which means that we'd have to drop all matching constraints, or something. The main source of nastiness, when we allow multiple constraints, is constraint inheritance. If we allow just one constraint per column, then it's always easy to know what to do on inheritance attach and detach: just coninhcount+1 or coninhcount-1 of the one relevant constraint (which can be matched by column name). If we have multiple ones, we have to know which one(s) to match and how (by constraint name?); if the parent has two and the child has one, we need to create another in the child, with its own coninhcount adjustments; if the parent has one named parent_col_not_null and the child also has child_col_not_null, then at ADD INHERIT do we match these ignoring the differing name, or do we rename the one on child so that we now have two? Also, the clutter in psql/pg_dump becomes worse. I would suggest that domain not-null constraints should also allow just one per column. -- Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/ "People get annoyed when you try to debug them." (Larry Wall)
wandering around the function AlterDomainNotNull,
the following code can fix the previous undesired behavior.
seems pretty simple, am I missing something?
based on v3-0001-Add-tests-for-domain-related-information-schema-v.patch
and v3-0002-Catalog-domain-not-null-constraints.patch
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index 2f94e375..9069465a 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -2904,7 +2904,7 @@ AlterDomainAddConstraint(List *names, Node *newConstraint,
Form_pg_type typTup;
Constraint *constr;
char *ccbin;
- ObjectAddress address;
+ ObjectAddress address = InvalidObjectAddress;
/* Make a TypeName so we can use standard type lookup machinery */
typename = makeTypeNameFromNameList(names);
@@ -3003,6 +3003,12 @@ AlterDomainAddConstraint(List *names, Node
*newConstraint,
}
else if (constr->contype == CONSTR_NOTNULL)
{
+ /* Is the domain already set NOT NULL */
+ if (typTup->typnotnull)
+ {
+ table_close(typrel, RowExclusiveLock);
+ return address;
+ }
domainAddNotNullConstraint(domainoid, typTup->typnamespace,
typTup->typbasetype, typTup->typtypmod,
constr, NameStr(typTup->typname), constrAddr);
On 12.02.24 11:24, Alvaro Herrera wrote:
> On 2024-Feb-11, Peter Eisentraut wrote:
>> But I see that table constraints do not work that way. A command like ALTER
>> TABLE t1 ADD NOT NULL c1 does nothing if the column already has a NOT NULL
>> constraint. I'm not sure this is correct. At least it's not documented.
>> We should probably make the domains feature work the same way, but I would
>> like to understand why it works that way first.
> The main source of nastiness, when we allow multiple constraints, is
> constraint inheritance. If we allow just one constraint per column,
> then it's always easy to know what to do on inheritance attach and
> detach: just coninhcount+1 or coninhcount-1 of the one relevant
> constraint (which can be matched by column name). If we have multiple
> ones, we have to know which one(s) to match and how (by constraint
> name?); if the parent has two and the child has one, we need to create
> another in the child, with its own coninhcount adjustments; if the
> parent has one named parent_col_not_null and the child also has
> child_col_not_null, then at ADD INHERIT do we match these ignoring the
> differing name, or do we rename the one on child so that we now have
> two? Also, the clutter in psql/pg_dump becomes worse.
>
> I would suggest that domain not-null constraints should also allow just
> one per column.
Perhaps it would make sense if we change the ALTER TABLE command to be like
ALTER TABLE t1 ADD IF NOT EXISTS NOT NULL c1
Then the behavior is like one would expect.
For ALTER TABLE, we would reject this command if IF NOT EXISTS is not
specified. (Since this is mainly for pg_dump, it doesn't really matter
for usability.) For ALTER DOMAIN, we could accept both variants.
On 2024-Mar-14, Peter Eisentraut wrote: > Perhaps it would make sense if we change the ALTER TABLE command to be like > > ALTER TABLE t1 ADD IF NOT EXISTS NOT NULL c1 > > Then the behavior is like one would expect. > > For ALTER TABLE, we would reject this command if IF NOT EXISTS is not > specified. (Since this is mainly for pg_dump, it doesn't really matter for > usability.) For ALTER DOMAIN, we could accept both variants. I don't understand why you want to change this behavior, though. -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ "La victoria es para quien se atreve a estar solo"
On 14.03.24 15:03, Alvaro Herrera wrote:
> On 2024-Mar-14, Peter Eisentraut wrote:
>
>> Perhaps it would make sense if we change the ALTER TABLE command to be like
>>
>> ALTER TABLE t1 ADD IF NOT EXISTS NOT NULL c1
>>
>> Then the behavior is like one would expect.
>>
>> For ALTER TABLE, we would reject this command if IF NOT EXISTS is not
>> specified. (Since this is mainly for pg_dump, it doesn't really matter for
>> usability.) For ALTER DOMAIN, we could accept both variants.
>
> I don't understand why you want to change this behavior, though.
Because in the abstract, the behavior of
ALTER TABLE t1 ADD <constraint specification>
should be to add a constraint.
In the current implementation, the behavior is different for different
constraint types.
Anyway, in order to move this forward, here is an updated patch where the ADD CONSTRAINT ... NOT NULL behavior for domains matches the idempotent behavior of tables. This uses the patch that Jian He posted.
Вложения
Hi, > Anyway, in order to move this forward, here is an updated patch where > the ADD CONSTRAINT ... NOT NULL behavior for domains matches the > idempotent behavior of tables. This uses the patch that Jian He posted. I tested the patch on Raspberry Pi 5 and Intel MacBook and also experimented with it. Everything seems to work properly. Personally I believe new functions such as validateDomainNotNullConstraint() and findDomainNotNullConstraint() could use a few lines of comments (accepts..., returns..., etc). Also I think that the commit message should explicitly say that supporting NOT VALID constraints is out of scope of this patch. Except for named nitpicks v4 LGTM. -- Best regards, Aleksander Alekseev
create domain connotnull integer; create table domconnotnulltest ( col1 connotnull , col2 connotnull ); alter domain connotnull add not null value; --------------------------- the above query does not work in pg16. ERROR: syntax error at or near "not". after applying the patch, now this works. this new syntax need to be added into the alter_domain.sgml's synopsis and also need an explanation varlistentry? + /* + * Store the constraint in pg_constraint + */ + ccoid = + CreateConstraintEntry(constr->conname, /* Constraint Name */ + domainNamespace, /* namespace */ + CONSTRAINT_NOTNULL, /* Constraint Type */ + false, /* Is Deferrable */ + false, /* Is Deferred */ + !constr->skip_validation, /* Is Validated */ + InvalidOid, /* no parent constraint */ + InvalidOid, /* not a relation constraint */ + NULL, + 0, + 0, + domainOid, /* domain constraint */ + InvalidOid, /* no associated index */ + InvalidOid, /* Foreign key fields */ + NULL, + NULL, + NULL, + NULL, + 0, + ' ', + ' ', + NULL, + 0, + ' ', + NULL, /* not an exclusion constraint */ + NULL, + NULL, + true, /* is local */ + 0, /* inhcount */ + false, /* connoinherit */ + false, /* conwithoutoverlaps */ + false); /* is_internal */ /* conwithoutoverlaps */ should be /* conperiod */
On 18.03.24 11:02, Aleksander Alekseev wrote: > Hi, > >> Anyway, in order to move this forward, here is an updated patch where >> the ADD CONSTRAINT ... NOT NULL behavior for domains matches the >> idempotent behavior of tables. This uses the patch that Jian He posted. > > I tested the patch on Raspberry Pi 5 and Intel MacBook and also > experimented with it. Everything seems to work properly. > > Personally I believe new functions such as > validateDomainNotNullConstraint() and findDomainNotNullConstraint() > could use a few lines of comments (accepts..., returns..., etc). Done. > Also > I think that the commit message should explicitly say that supporting > NOT VALID constraints is out of scope of this patch. Not done. I don't know what NOT VALID has to do with this. This patch just changes the internal catalog representation, it doesn't claim to add or change any features. The documentation already accurately states that NOT VALID is not supported for NOT NULL constraints. > Except for named nitpicks v4 LGTM. Committed, thanks.
On 19.03.24 10:57, jian he wrote: > this new syntax need to be added into the alter_domain.sgml's synopsis and also > need an explanation varlistentry? The ALTER DOMAIN reference page refers to CREATE DOMAIN about the details of the constraint syntax. I believe this is still accurate. We could add more detail locally on the ALTER DOMAIN page, but that is not this patch's job. For example, the details of CHECK constraints are also not shown on the ALTER DOMAIN page right now. > + false, /* connoinherit */ > + false, /* conwithoutoverlaps */ > + false); /* is_internal */ > > /* conwithoutoverlaps */ > should be > /* conperiod */ Good catch, thanks.
On Wed, 20 Mar 2024 at 09:43, Peter Eisentraut <peter@eisentraut.org> wrote:
>
> On 19.03.24 10:57, jian he wrote:
> > this new syntax need to be added into the alter_domain.sgml's synopsis and also
> > need an explanation varlistentry?
>
> The ALTER DOMAIN reference page refers to CREATE DOMAIN about the
> details of the constraint syntax. I believe this is still accurate. We
> could add more detail locally on the ALTER DOMAIN page, but that is not
> this patch's job. For example, the details of CHECK constraints are
> also not shown on the ALTER DOMAIN page right now.
>
Hmm, for CHECK constraints, the ALTER DOMAIN syntax for adding a
constraint is the same as for CREATE DOMAIN, but that's not the case
for NOT NULL constraints. So, for example, these both work:
CREATE DOMAIN d AS int CONSTRAINT c1 CHECK (value > 0);
ALTER DOMAIN d ADD CONSTRAINT c2 CHECK (value < 10);
However, for NOT NULL constraints, the ALTER DOMAIN syntax differs
from the CREATE DOMAIN syntax, because it expects "NOT NULL" to be
followed by a column name. So the following CREATE DOMAIN syntax
works:
CREATE DOMAIN d AS int CONSTRAINT nn NOT NULL;
but the equivalent ALTER DOMAIN syntax doesn't work:
ALTER DOMAIN d ADD CONSTRAINT nn NOT NULL;
ERROR: syntax error at or near ";"
LINE 1: ALTER DOMAIN d ADD CONSTRAINT nn NOT NULL;
^
All the examples in the tests append "value" to this, presumably by
analogy with CHECK constraints, but it looks as though anything works,
and is simply ignored:
ALTER DOMAIN d ADD CONSTRAINT nn NOT NULL xxx; -- works
That doesn't seem particularly satisfactory. I think it should not
require (and reject) a column name after "NOT NULL".
Looking in the SQL spec, it seems to only mention adding CHECK
constraints to domains, so the option to add NOT NULL constraints
should probably be listed in the "Compatibility" section.
Regards,
Dean
On 20.03.24 12:22, Dean Rasheed wrote: > Hmm, for CHECK constraints, the ALTER DOMAIN syntax for adding a > constraint is the same as for CREATE DOMAIN, but that's not the case > for NOT NULL constraints. So, for example, these both work: > > CREATE DOMAIN d AS int CONSTRAINT c1 CHECK (value > 0); > > ALTER DOMAIN d ADD CONSTRAINT c2 CHECK (value < 10); > > However, for NOT NULL constraints, the ALTER DOMAIN syntax differs > from the CREATE DOMAIN syntax, because it expects "NOT NULL" to be > followed by a column name. So the following CREATE DOMAIN syntax > works: > > CREATE DOMAIN d AS int CONSTRAINT nn NOT NULL; > > but the equivalent ALTER DOMAIN syntax doesn't work: > > ALTER DOMAIN d ADD CONSTRAINT nn NOT NULL; > > ERROR: syntax error at or near ";" > LINE 1: ALTER DOMAIN d ADD CONSTRAINT nn NOT NULL; > ^ > > All the examples in the tests append "value" to this, presumably by > analogy with CHECK constraints, but it looks as though anything works, > and is simply ignored: > > ALTER DOMAIN d ADD CONSTRAINT nn NOT NULL xxx; -- works > > That doesn't seem particularly satisfactory. I think it should not > require (and reject) a column name after "NOT NULL". Hmm. CREATE DOMAIN uses column constraint syntax, but ALTER DOMAIN uses table constraint syntax. As long as you are only dealing with CHECK constraints, there is no difference, but it shows up when using NOT NULL constraint syntax. I agree that this is unsatisfactory. Attached is a patch to try to sort this out. > Looking in the SQL spec, it seems to only mention adding CHECK > constraints to domains, so the option to add NOT NULL constraints > should probably be listed in the "Compatibility" section. <canofworms> A quick reading of the SQL standard suggests to me that the way we are doing null handling in domain constraints is all wrong. The standard says that domain constraints are only checked on values that are not null. So both the handling of constraints using the CHECK syntax is nonstandard and the existence of explicit NOT NULL constraints is an extension. The CREATE DOMAIN reference page already explains why all of this is a bad idea. Do we want to document all of that further, or maybe we just want to rip out domain not-null constraints, or at least not add further syntax for it? </canofworms>
Вложения
Peter Eisentraut <peter@eisentraut.org> writes:
> <canofworms>
> A quick reading of the SQL standard suggests to me that the way we are
> doing null handling in domain constraints is all wrong. The standard
> says that domain constraints are only checked on values that are not
> null. So both the handling of constraints using the CHECK syntax is
> nonstandard and the existence of explicit NOT NULL constraints is an
> extension. The CREATE DOMAIN reference page already explains why all of
> this is a bad idea. Do we want to document all of that further, or
> maybe we just want to rip out domain not-null constraints, or at least
> not add further syntax for it?
> </canofworms>
Yeah. The real problem with domain not null is: how can a column
that's propagated up through the nullable side of an outer join
still be considered to belong to such a domain?
The SQL spec's answer to that conundrum appears to be "NULL is
a valid value of every domain, and if you don't like it, tough".
I'm too lazy to search the archives, but we have had at least one
previous discussion about how we should adopt the spec's semantics.
It'd be an absolutely trivial fix in CoerceToDomain (succeed
immediately if input is NULL), but the question is what to do
with existing "DOMAIN NOT NULL" DDL.
Anyway, now that I recall all that, e5da0fe3c is throwing good work
after bad, and I wonder if we shouldn't revert it.
regards, tom lane
On Thu, 21 Mar 2024 at 10:30, Tom Lane <tgl@sss.pgh.pa.us> wrote:
The SQL spec's answer to that conundrum appears to be "NULL is
a valid value of every domain, and if you don't like it, tough".
To be fair, NULL is a valid value of every type. Even VOID has NULL.
In this context, it’s a bit weird to be able to decree up front when defining a type that no table column of that type, anywhere, may ever contain a NULL. It would be nice if there was a way to reverse the default so that if you (almost or) never want NULLs anywhere that’s what you get without saying "NOT NULL" all over the place, and instead just specify "NULLABLE" (or something) where you want. But that effectively means optionally changing the behaviour of CREATE TABLE and ALTER TABLE.
On 3/21/24 15:30, Tom Lane wrote: > Peter Eisentraut <peter@eisentraut.org> writes: >> <canofworms> >> A quick reading of the SQL standard suggests to me that the way we are >> doing null handling in domain constraints is all wrong. The standard >> says that domain constraints are only checked on values that are not >> null. So both the handling of constraints using the CHECK syntax is >> nonstandard and the existence of explicit NOT NULL constraints is an >> extension. The CREATE DOMAIN reference page already explains why all of >> this is a bad idea. Do we want to document all of that further, or >> maybe we just want to rip out domain not-null constraints, or at least >> not add further syntax for it? >> </canofworms> > > Yeah. The real problem with domain not null is: how can a column > that's propagated up through the nullable side of an outer join > still be considered to belong to such a domain? Per spec, it is not considered to be so. The domain only applies to table storage and CASTs and gets "forgotten" in a query. > The SQL spec's answer to that conundrum appears to be "NULL is > a valid value of every domain, and if you don't like it, tough". I don't see how you can infer this from the standard at all. > I'm too lazy to search the archives, but we have had at least one > previous discussion about how we should adopt the spec's semantics. > It'd be an absolutely trivial fix in CoerceToDomain (succeed > immediately if input is NULL), but the question is what to do > with existing "DOMAIN NOT NULL" DDL. Here is a semi-random link into a conversation you and I have recently had about this: https://www.postgresql.org/message-id/a13db59c-c68f-4a30-87a5-177fe135665e%40postgresfriends.org As also said somewhere in that thread, I think that <cast specification> short-cutting a NULL input value without considering the constraints of a domain is a bug that needs to be fixed in the standard. -- Vik Fearing
Vik Fearing <vik@postgresfriends.org> writes:
> On 3/21/24 15:30, Tom Lane wrote:
>> The SQL spec's answer to that conundrum appears to be "NULL is
>> a valid value of every domain, and if you don't like it, tough".
> I don't see how you can infer this from the standard at all.
I believe where we got that from is 6.13 <cast specification>,
which quoth (general rule 2):
c) If SV is the null value, then the result of CS is the null
value and no further General Rules of this Subclause are applied.
In particular, that short-circuits application of the domain
constraints (GR 23), implying that CAST(NULL AS some_domain) is
always successful. Now you could argue that there's some other
context that would reject nulls, but being inconsistent with
CAST would seem more like a bug than a feature.
> As also said somewhere in that thread, I think that <cast specification>
> short-cutting a NULL input value without considering the constraints of
> a domain is a bug that needs to be fixed in the standard.
I think it's probably intentional. It certainly fits with the lack of
syntax for DOMAIN NOT NULL. Also, it's been like that since SQL99;
do you think nobody's noticed it for 25 years?
regards, tom lane
On 3/22/24 00:17, Tom Lane wrote: > Vik Fearing <vik@postgresfriends.org> writes: >> On 3/21/24 15:30, Tom Lane wrote: >>> The SQL spec's answer to that conundrum appears to be "NULL is >>> a valid value of every domain, and if you don't like it, tough". > >> I don't see how you can infer this from the standard at all. > > I believe where we got that from is 6.13 <cast specification>, > which quoth (general rule 2): > > c) If SV is the null value, then the result of CS is the null > value and no further General Rules of this Subclause are applied. > > In particular, that short-circuits application of the domain > constraints (GR 23), implying that CAST(NULL AS some_domain) is > always successful. Now you could argue that there's some other > context that would reject nulls, but being inconsistent with > CAST would seem more like a bug than a feature. I think the main bug is in what you quoted from <cast specification>. I believe that the POLA for casting to a domain is for all constraints of the domain to be verified for ALL values including the null value. >> As also said somewhere in that thread, I think that <cast specification> >> short-cutting a NULL input value without considering the constraints of >> a domain is a bug that needs to be fixed in the standard. > > I think it's probably intentional. It certainly fits with the lack of > syntax for DOMAIN NOT NULL. Also, it's been like that since SQL99; > do you think nobody's noticed it for 25 years? Haven't we (postgres) had bug reports of similar age? There is also the possibility that no one has noticed because major players have not implemented domains. For example, Oracle only just got them last year: https://blogs.oracle.com/coretec/post/less-coding-with-sql-domains-in-23c Anyway, I will bring this up with the committee and report back. My proposed solution will be for CAST to check domain constraints even if the input is NULL. -- Vik Fearing
Vik Fearing <vik@postgresfriends.org> writes:
> On 3/22/24 00:17, Tom Lane wrote:
>> Vik Fearing <vik@postgresfriends.org> writes:
>>> As also said somewhere in that thread, I think that <cast specification>
>>> short-cutting a NULL input value without considering the constraints of
>>> a domain is a bug that needs to be fixed in the standard.
>> I think it's probably intentional. It certainly fits with the lack of
>> syntax for DOMAIN NOT NULL. Also, it's been like that since SQL99;
>> do you think nobody's noticed it for 25 years?
> Haven't we (postgres) had bug reports of similar age?
Well, they've looked it at it since then. SQL99 has
c) If SV is the null value, then the result is the null value.
SQL:2008 and later have the text I quoted:
c) If SV is the null value, then the result of CS is the null
value and no further General Rules of this Sub-clause are
applied.
I find it *extremely* hard to believe that they would have added
that explicit text without noticing exactly which operations they
were saying to skip.
> Anyway, I will bring this up with the committee and report back. My
> proposed solution will be for CAST to check domain constraints even if
> the input is NULL.
Please do not claim that that is the position of the Postgres project.
regards, tom lane
On 3/22/24 01:46, Tom Lane wrote: > Vik Fearing <vik@postgresfriends.org> writes: >> Anyway, I will bring this up with the committee and report back. My >> proposed solution will be for CAST to check domain constraints even if >> the input is NULL. > > Please do not claim that that is the position of the Postgres project. Everything that I do on the SQL Committee is in my own name. I do not speak for either PostgreSQL or for EDB (my employer), even though my opinions are of course often influenced by some combination of both. -- Vik Fearing
On Thu, Mar 21, 2024 at 7:23 PM Peter Eisentraut <peter@eisentraut.org> wrote:
>
> On 20.03.24 12:22, Dean Rasheed wrote:
> > Hmm, for CHECK constraints, the ALTER DOMAIN syntax for adding a
> > constraint is the same as for CREATE DOMAIN, but that's not the case
> > for NOT NULL constraints. So, for example, these both work:
> >
> > CREATE DOMAIN d AS int CONSTRAINT c1 CHECK (value > 0);
> >
> > ALTER DOMAIN d ADD CONSTRAINT c2 CHECK (value < 10);
> >
> > However, for NOT NULL constraints, the ALTER DOMAIN syntax differs
> > from the CREATE DOMAIN syntax, because it expects "NOT NULL" to be
> > followed by a column name. So the following CREATE DOMAIN syntax
> > works:
> >
> > CREATE DOMAIN d AS int CONSTRAINT nn NOT NULL;
> >
> > but the equivalent ALTER DOMAIN syntax doesn't work:
> >
> > ALTER DOMAIN d ADD CONSTRAINT nn NOT NULL;
> >
> > ERROR: syntax error at or near ";"
> > LINE 1: ALTER DOMAIN d ADD CONSTRAINT nn NOT NULL;
> > ^
> >
> > All the examples in the tests append "value" to this, presumably by
> > analogy with CHECK constraints, but it looks as though anything works,
> > and is simply ignored:
> >
> > ALTER DOMAIN d ADD CONSTRAINT nn NOT NULL xxx; -- works
> >
> > That doesn't seem particularly satisfactory. I think it should not
> > require (and reject) a column name after "NOT NULL".
>
> Hmm. CREATE DOMAIN uses column constraint syntax, but ALTER DOMAIN uses
> table constraint syntax. As long as you are only dealing with CHECK
> constraints, there is no difference, but it shows up when using NOT NULL
> constraint syntax. I agree that this is unsatisfactory. Attached is a
> patch to try to sort this out.
>
+ | NOT NULL_P ConstraintAttributeSpec
+ {
+ Constraint *n = makeNode(Constraint);
+
+ n->contype = CONSTR_NOTNULL;
+ n->location = @1;
+ n->keys = list_make1(makeString("value"));
+ /* no NOT VALID support yet */
+ processCASbits($3, @3, "NOT NULL",
+ NULL, NULL, NULL,
+ &n->is_no_inherit, yyscanner);
+ n->initially_valid = true;
+ $$ = (Node *) n;
+ }
i don't understand this part.
+ n->keys = list_make1(makeString("value"));
also you should also change src/backend/utils/adt/ruleutils.c?
src6=# create domain domain_test integer;
alter domain domain_test add constraint pos1 check (value > 0);
alter domain domain_test add constraint constr1 not null ;
CREATE DOMAIN
ALTER DOMAIN
ALTER DOMAIN
src6=# \dD
List of domains
Schema | Name | Type | Collation | Nullable | Default |
Check
--------+-------------+---------+-----------+----------+---------+----------------------------------
public | domain_test | integer | | not null | |
CHECK (VALUE > 0) NOT NULL VALUE
(1 row)
probably change to CHECK (VALUE IS NOT NULL)
- appendStringInfoString(&buf,
"NOT NULL VALUE");
+ appendStringInfoString(&buf,
"CHECK (VALUE IS NOT NULL)");
seems works.
On Fri, 22 Mar 2024 at 08:28, jian he <jian.universality@gmail.com> wrote: > > On Thu, Mar 21, 2024 at 7:23 PM Peter Eisentraut <peter@eisentraut.org> wrote: > > > > Hmm. CREATE DOMAIN uses column constraint syntax, but ALTER DOMAIN uses > > table constraint syntax. Attached is a patch to try to sort this out. > > also you should also change src/backend/utils/adt/ruleutils.c? > > src6=# \dD > List of domains > Schema | Name | Type | Collation | Nullable | Default | > Check > --------+-------------+---------+-----------+----------+---------+---------------------------------- > public | domain_test | integer | | not null | | > CHECK (VALUE > 0) NOT NULL VALUE > (1 row) > > probably change to CHECK (VALUE IS NOT NULL) I'd say it should just output "NOT NULL", since that's the input syntax that created the constraint. But then again, why display NOT NULL constraints in that column at all, when there's a separate "Nullable" column? Also (not this patch's fault), psql doesn't seem to offer a way to display domain constraint names -- something you need to know to drop or alter them. Perhaps \dD+ could be made to do that? + The syntax <literal>NOT NULL</literal> in this command is a + <productname>PostgreSQL</productname> extension. (A standard-conforming + way to write the same would be <literal>CHECK (VALUE IS NOT + NULL)</literal>. However, per <xref linkend="sql-createdomain-notes"/>, + such constraints a best avoided in practice anyway.) The + <literal>NULL</literal> <quote>constraint</quote> is a + <productname>PostgreSQL</productname> extension (see also <xref + linkend="sql-createtable-compatibility"/>). I didn't verify this, but I thought that according to the SQL standard, only non-NULL values should be passed to CHECK constraints, so there is no standard-conforming way to write a NOT NULL domain constraint. FWIW, I think NOT NULL domain constraints are a useful feature to have, and I suspect that there are more people out there who use them and like them, than who care what the SQL standard says. If so, I'm in favour of allowing them to be named and managed in the same way as NOT NULL table constraints. + processCASbits($5, @5, "CHECK", + NULL, NULL, &n->skip_validation, + &n->is_no_inherit, yyscanner); + n->initially_valid = !n->skip_validation; + /* no NOT VALID support yet */ + processCASbits($3, @3, "NOT NULL", + NULL, NULL, NULL, + &n->is_no_inherit, yyscanner); + n->initially_valid = true; NO INHERIT is allowed for domain constraints? What does that even mean? There's something very wonky about this: CREATE DOMAIN d1 AS int CHECK (value > 0) NO INHERIT; -- Rejected ERROR: check constraints for domains cannot be marked NO INHERIT CREATE DOMAIN d1 AS int; ALTER DOMAIN d1 ADD CHECK (value > 0) NO INHERIT; -- Allowed CREATE DOMAIN d2 AS int NOT NULL NO INHERIT; -- Now allowed (used to syntax error) CREATE DOMAIN d3 AS int; ALTER DOMAIN d3 ADD NOT NULL NO INHERIT; -- Allowed Presumably all of those should be rejected in the grammar. Regards, Dean
On 2024-Mar-25, Dean Rasheed wrote: > Also (not this patch's fault), psql doesn't seem to offer a way to > display domain constraint names -- something you need to know to drop > or alter them. Perhaps \dD+ could be made to do that? Ooh, I remember we had offered a patch for \d++ to display these constraint names for tables, but didn't get around to gather consensus for it. We did gather consensus on *not* wanting \d+ to display them, but we need *something*. I suppose we should do something symmetrical for tables and domains. How about \dD++ and \dt++? -- Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/ Bob [Floyd] used to say that he was planning to get a Ph.D. by the "green stamp method," namely by saving envelopes addressed to him as 'Dr. Floyd'. After collecting 500 such letters, he mused, a university somewhere in Arizona would probably grant him a degree. (Don Knuth)
On Tue, 26 Mar 2024 at 07:30, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> On 2024-Mar-25, Dean Rasheed wrote:
>
> > Also (not this patch's fault), psql doesn't seem to offer a way to
> > display domain constraint names -- something you need to know to drop
> > or alter them. Perhaps \dD+ could be made to do that?
>
> Ooh, I remember we had offered a patch for \d++ to display these
> constraint names for tables, but didn't get around to gather consensus
> for it. We did gather consensus on *not* wanting \d+ to display them,
> but we need *something*. I suppose we should do something symmetrical
> for tables and domains. How about \dD++ and \dt++?
>
Personally, I quite like the fact that \d+ displays NOT NULL
constraints, because it puts them on an equal footing with CHECK
constraints. However, I can appreciate that it will significantly
increase the length of the output in some cases.
With \dD it's not so nice because of the way it puts all the details
on one line. The obvious output might look something like this:
\dD
List of domains
Schema | Name | Type | Collation | Nullable | Default | Check
--------+------+---------+-----------+----------+---------+-------------------
public | d1 | integer | | NOT NULL | | CHECK (VALUE > 0)
\dD+
List of domains
Schema | Name | Type | Collation | Nullable
| Default | Check | Access privileges
| Description
--------+------+---------+-----------+---------------------------------+---------+---------------------------------------+-------------------+-------------
public | d1 | integer | | CONSTRAINT d1_not_null NOT NULL
| | CONSTRAINT d1_check CHECK (VALUE > 0) |
|
So you'd need quite a wide window to easily view it (or use \x). I
suppose the width could be reduced by dropping the word "CONSTRAINT"
in the \dD+ case, but it's probably still going to be wider than the
average window.
Regards,
Dean
On Tue, Mar 26, 2024 at 2:28 AM Dean Rasheed <dean.a.rasheed@gmail.com> wrote: > > On Fri, 22 Mar 2024 at 08:28, jian he <jian.universality@gmail.com> wrote: > > > > On Thu, Mar 21, 2024 at 7:23 PM Peter Eisentraut <peter@eisentraut.org> wrote: > > > > > > Hmm. CREATE DOMAIN uses column constraint syntax, but ALTER DOMAIN uses > > > table constraint syntax. Attached is a patch to try to sort this out. > > > > also you should also change src/backend/utils/adt/ruleutils.c? > > > > src6=# \dD > > List of domains > > Schema | Name | Type | Collation | Nullable | Default | > > Check > > --------+-------------+---------+-----------+----------+---------+---------------------------------- > > public | domain_test | integer | | not null | | > > CHECK (VALUE > 0) NOT NULL VALUE > > (1 row) > > > > probably change to CHECK (VALUE IS NOT NULL) > > I'd say it should just output "NOT NULL", since that's the input > syntax that created the constraint. But then again, why display NOT > NULL constraints in that column at all, when there's a separate > "Nullable" column? > create table sss(a int not null); SELECT pg_get_constraintdef(oid) FROM pg_constraint WHERE conname = 'sss_a_not_null'; returns " NOT NULL a" I think just outputting "NOT NULL" is ok for the domain, given the table constraint is "NOT NULL" + table column, per above example. yech, we already have a "Nullable" column, so we don't need to display NOT NULL constraints.
On 21.03.24 12:23, Peter Eisentraut wrote: >> All the examples in the tests append "value" to this, presumably by >> analogy with CHECK constraints, but it looks as though anything works, >> and is simply ignored: >> >> ALTER DOMAIN d ADD CONSTRAINT nn NOT NULL xxx; -- works >> >> That doesn't seem particularly satisfactory. I think it should not >> require (and reject) a column name after "NOT NULL". > > Hmm. CREATE DOMAIN uses column constraint syntax, but ALTER DOMAIN uses > table constraint syntax. As long as you are only dealing with CHECK > constraints, there is no difference, but it shows up when using NOT NULL > constraint syntax. I agree that this is unsatisfactory. Attached is a > patch to try to sort this out. After studying this a bit more, I think moving forward in this direction is the best way. Attached is a new patch version, mainly with a more elaborate commit message. This patch makes the not-null constraint syntax consistent between CREATE DOMAIN and ALTER DOMAIN, and also makes the respective documentation correct. (Note that, as I show in the commit message, commit e5da0fe3c22 had in passing fixed a couple of bugs in CREATE and ALTER DOMAIN, so just reverting that commit wouldn't be a complete solution.)
Вложения
On Mon, Apr 8, 2024 at 5:53 PM Peter Eisentraut <peter@eisentraut.org> wrote:
>
> On 21.03.24 12:23, Peter Eisentraut wrote:
> >> All the examples in the tests append "value" to this, presumably by
> >> analogy with CHECK constraints, but it looks as though anything works,
> >> and is simply ignored:
> >>
> >> ALTER DOMAIN d ADD CONSTRAINT nn NOT NULL xxx; -- works
> >>
> >> That doesn't seem particularly satisfactory. I think it should not
> >> require (and reject) a column name after "NOT NULL".
> >
> > Hmm. CREATE DOMAIN uses column constraint syntax, but ALTER DOMAIN uses
> > table constraint syntax. As long as you are only dealing with CHECK
> > constraints, there is no difference, but it shows up when using NOT NULL
> > constraint syntax. I agree that this is unsatisfactory. Attached is a
> > patch to try to sort this out.
>
> After studying this a bit more, I think moving forward in this direction
> is the best way. Attached is a new patch version, mainly with a more
> elaborate commit message. This patch makes the not-null constraint
> syntax consistent between CREATE DOMAIN and ALTER DOMAIN, and also makes
> the respective documentation correct.
>
> (Note that, as I show in the commit message, commit e5da0fe3c22 had in
> passing fixed a couple of bugs in CREATE and ALTER DOMAIN, so just
> reverting that commit wouldn't be a complete solution.)
in ruleutils.c
/* conkey is null for domain not-null constraints */
appendStringInfoString(&buf, "NOT NULL VALUE");
should be
/* conkey is null for domain not-null constraints */
appendStringInfoString(&buf, "NOT NULL ");
?
currently
src6=# \dD connotnull
/******** QUERY *********/
SELECT n.nspname as "Schema",
t.typname as "Name",
pg_catalog.format_type(t.typbasetype, t.typtypmod) as "Type",
(SELECT c.collname FROM pg_catalog.pg_collation c, pg_catalog.pg_type bt
WHERE c.oid = t.typcollation AND bt.oid = t.typbasetype AND
t.typcollation <> bt.typcollation) as "Collation",
CASE WHEN t.typnotnull THEN 'not null' END as "Nullable",
t.typdefault as "Default",
pg_catalog.array_to_string(ARRAY(
SELECT pg_catalog.pg_get_constraintdef(r.oid, true) FROM
pg_catalog.pg_constraint r WHERE t.oid = r.contypid
), ' ') as "Check"
FROM pg_catalog.pg_type t
LEFT JOIN pg_catalog.pg_namespace n ON n.oid = t.typnamespace
WHERE t.typtype = 'd'
AND t.typname OPERATOR(pg_catalog.~) '^(connotnull)$' COLLATE
pg_catalog.default
AND pg_catalog.pg_type_is_visible(t.oid)
ORDER BY 1, 2;
/************************/
---
Since the last column is already named as "Check", maybe we need to
change the query to
pg_catalog.array_to_string(ARRAY(
SELECT pg_catalog.pg_get_constraintdef(r.oid, true) FROM
pg_catalog.pg_constraint r WHERE t.oid = r.contypid
and r.contype = 'c'
), ' ') as "Check"
That means domain can be associated with check constraint and not-null
constraint.
the url link destination is fine, but the url rendered name is "per
the section called “Notes”" which seems strange,
please see attached.
Вложения
On 09.04.24 10:44, jian he wrote: >> After studying this a bit more, I think moving forward in this direction >> is the best way. Attached is a new patch version, mainly with a more >> elaborate commit message. This patch makes the not-null constraint >> syntax consistent between CREATE DOMAIN and ALTER DOMAIN, and also makes >> the respective documentation correct. >> >> (Note that, as I show in the commit message, commit e5da0fe3c22 had in >> passing fixed a couple of bugs in CREATE and ALTER DOMAIN, so just >> reverting that commit wouldn't be a complete solution.) > in ruleutils.c > /* conkey is null for domain not-null constraints */ > appendStringInfoString(&buf, "NOT NULL VALUE"); > > should be > > /* conkey is null for domain not-null constraints */ > appendStringInfoString(&buf, "NOT NULL "); Good catch, fixed. > currently > src6=# \dD connotnull > /******** QUERY *********/ > SELECT n.nspname as "Schema", > t.typname as "Name", > pg_catalog.format_type(t.typbasetype, t.typtypmod) as "Type", > (SELECT c.collname FROM pg_catalog.pg_collation c, pg_catalog.pg_type bt > WHERE c.oid = t.typcollation AND bt.oid = t.typbasetype AND > t.typcollation <> bt.typcollation) as "Collation", > CASE WHEN t.typnotnull THEN 'not null' END as "Nullable", > t.typdefault as "Default", > pg_catalog.array_to_string(ARRAY( > SELECT pg_catalog.pg_get_constraintdef(r.oid, true) FROM > pg_catalog.pg_constraint r WHERE t.oid = r.contypid > ), ' ') as "Check" > FROM pg_catalog.pg_type t > LEFT JOIN pg_catalog.pg_namespace n ON n.oid = t.typnamespace > WHERE t.typtype = 'd' > AND t.typname OPERATOR(pg_catalog.~) '^(connotnull)$' COLLATE > pg_catalog.default > AND pg_catalog.pg_type_is_visible(t.oid) > ORDER BY 1, 2; > /************************/ > > --- > Since the last column is already named as "Check", maybe we need to > change the query to > pg_catalog.array_to_string(ARRAY( > SELECT pg_catalog.pg_get_constraintdef(r.oid, true) FROM > pg_catalog.pg_constraint r WHERE t.oid = r.contypid > and r.contype = 'c' > ), ' ') as "Check" > > That means domain can be associated with check constraint and not-null > constraint. Yes, I changed it like you wrote. > the url link destination is fine, but the url rendered name is "per > the section called “Notes”" which seems strange, > please see attached. Hmm, changing that would be an independent project. I have committed the patch with the two amendments you provided. I had also added a test of \dD and that caused some test output instability, so I added a ORDER BY r.conname to the \dD query.