Re: Catalog domain not-null constraints

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.