Re: NOT ENFORCED constraint feature

From: Amul Sul <sulamul(at)gmail(dot)com>
To: jian he <jian(dot)universality(at)gmail(dot)com>
Cc: Peter Eisentraut <peter(at)eisentraut(dot)org>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Joel Jacobson <joel(at)compiler(dot)org>
Subject: Re: NOT ENFORCED constraint feature
Date: 2024-12-06 09:13:51
Message-ID: CAAJ_b970q6C1SVtfY-GEe7t2bfg381pLQ8Bm1q1q0y=7HOm30A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Dec 5, 2024 at 11:02 AM jian he <jian(dot)universality(at)gmail(dot)com> wrote:
>
> hi.
> accidentally hit segfault.
> create table c11 (a int not enforced);
> create table c11 (a int enforced);
> we can solve it via the following or changing SUPPORTS_ATTRS accordingly.
>
> diff --git a/src/backend/parser/parse_utilcmd.c
> b/src/backend/parser/parse_utilcmd.c
> index 5ab44149e5..fe1116c092 100644
> --- a/src/backend/parser/parse_utilcmd.c
> +++ b/src/backend/parser/parse_utilcmd.c
> @@ -3965,7 +3965,7 @@ transformConstraintAttrs(CreateStmtContext *cxt,
> List *constraintList)
> break;
> case CONSTR_ATTR_ENFORCED:
> - if (lastprimarycon &&
> + if (lastprimarycon == NULL ||
> lastprimarycon->contype != CONSTR_CHECK)
> ereport(ERROR,
>
> (errcode(ERRCODE_SYNTAX_ERROR),
> @@ -3981,7 +3981,7 @@ transformConstraintAttrs(CreateStmtContext *cxt,
> List *constraintList)
> break;
> case CONSTR_ATTR_NOT_ENFORCED:
> - if (lastprimarycon &&
> + if (lastprimarycon == NULL ||
> lastprimarycon->contype != CONSTR_CHECK)
> ereport(ERROR,
>
> (errcode(ERRCODE_SYNTAX_ERROR),
>

Yes, that was a logical oversight on my part. Your suggestion looks
good to me, thanks.

>
> ALTER DOMAIN constraint_comments_dom ADD CONSTRAINT the_constraint
> CHECK (value > 0) NOT ENFORCED;
> ERROR: CHECK constraints cannot be marked NOT ENFORCED
>
> the error message is not good? maybe better option would be:
> ERROR: DOMAIN CHECK constraints cannot be marked NOT ENFORCED
>
> we can do it like:
> index 833b3be02b..4a7ab0c2a3 100644
> --- a/src/backend/parser/gram.y
> +++ b/src/backend/parser/gram.y
> @@ -4342,7 +4342,7 @@ DomainConstraintElem:
> n->location = @1;
> n->raw_expr = $3;
> n->cooked_expr = NULL;
> - processCASbits($5, @5, "CHECK",
> + processCASbits($5, @5, "DOMAIN CHECK",
>
> NULL, NULL, NULL, &n->skip_validation,
>
> &n->is_no_inherit, yyscanner);

I believe this should either be a separate patch or potentially
included in your "Refactor AlterDomainAddConstraint" proposal[1].

Regards,
Amul

1] https://postgr.es/m/CACJufxHitd5LGLBSSAPShhtDWxT0ViVKTHinkYW-skBX93TcpA@mail.gmail.com

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Vaijayanti Bharadwaj 2024-12-06 09:20:01 logical replication: patch to ensure timely cleanup of aborted transactions in ReorderBuffer
Previous Message Daniel Gustafsson 2024-12-06 08:49:24 Re: Fix tiny memory leaks