Re: [PATCH] no table rewrite when set column type to constrained domain

On 24 Mar 2026 at 02:17 +0100, jian he <jian(dot)universality(at)gmail(dot)com>, wrote:
> On Tue, Mar 24, 2026 at 12:18 AM Viktor Holmberg <v(at)viktorh(dot)net> wrote:
> >
> > This appears to address some of my comments but not this one?
> >
> > CREATE DOMAIN domain1 AS INT CHECK(VALUE > 1) NOT NULL;
> > CREATE TABLE t_const_using(a INT);
> > INSERT INTO t_const_using VALUES(-2);
> > ALTER TABLE t_const_using ALTER COLUMN a SET DATA TYPE domain1 USING 5;
> > SELECT a FROM t_const_using; -- should be 5 after rewrite, not -2
> > a
> > ----
> > -2
>
> Sure, these tests are added to v6.
>
> --
> jian
> https://www.enterprisedb.com/
@@ -6077,7 +6084,7 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode,
 * rebuild data.
 */
 if (tab->constraints != NIL || tab->verify_new_notnull ||
- tab->partition_constraint != NULL)
+ tab->partition_constraint != NULL || tab->newvals)
ATRewriteTable(tab, InvalidOid);

I’m not 100% sure, but I think when you add an unconstraint domain:
CREATE DOMAIN mydom AS int; -- no CHECK, no NOT NULL
 CREATE TABLE t (a int);
 ALTER TABLE t ALTER COLUMN a TYPE mydom;

The tab->newvals check makes it so that ATRewriteTable is run, even though it’s not needed.

---
/*
 * ExecEvalExprNoReturn cannot be used here because
 * the expression was compiled via ExecInitExpr.
 */
 (void) ExecEvalExpr(ex->exprstate, econtext, &isnull);

I still don’t understand this comment at all. Not saying it’s wrong, but not sure if it’s right or not. Perhaps its clear for those that are more in the know so maybe if you can explain in an email it’s be enough.

---
Nit:
-- Test chaning  column data type to constrained domain

 "chaning" → "changing", and there's a double space.