| From: | jian he <jian(dot)universality(at)gmail(dot)com> |
|---|---|
| To: | Viktor Holmberg <v(at)viktorh(dot)net> |
| Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: [PATCH] no table rewrite when set column type to constrained domain |
| Date: | 2026-03-17 08:05:58 |
| Message-ID: | CACJufxHdv4UTJ9XZT0-89Z_a8QPrXr-=1WCyZ2htZ5dZVYH-GA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Sun, Mar 15, 2026 at 12:32 AM Viktor Holmberg <v(at)viktorh(dot)net> wrote:
>
> As I’m sure you know Jian this needs a rebase now that a0b6ef29a518 has been merged. It’s a bit hard for me to review in this state when I can’t apply the patches cleanly.
>
> + /*
> + * we can not use ExecEvalExprNoReturn here, because we
> + * use ExecInitExpr compile NewColumnValue->expr. Here,
> + * we only check whether the oldslot value satisfies the
> + * domain constraint. So it is ok to override the value
> + * evaluated by ExecEvalExpr.
> + */
> + values = ExecEvalExpr(ex->exprstate, econtext, &isnull);
> + values = (Datum) 0;
> + isnull = true;
>
> I don’t understand this piece of code, and why value is re-assigned right away. Not saying it’s wrong but if you could explain why it is like that to someone not well versed in C. Would something like (void) ExecEvalExpr(ex->exprstate, econtext, &isnull); do?
>
CREATE TABLE t(a int);
INSERT INTO t VALUES (1);
CREATE DOMAIN d1 AS INT check (value <> 1);
ALTER TABLE t ALTER COLUMN a SET DATA TYPE d1;
ATPrepAlterColumnType->coerce_to_target_type will return a COERCETODOMAIN Node
{COERCETODOMAIN
:arg
{VAR
:varno 1
:varattno 1
:vartype 23
:vartypmod -1
:varcollid 0
:varnullingrels (b)
:varlevelsup 0
:varreturningtype 0
:varnosyn 1
:varattnosyn 1
:location -1
}
:resulttype 18235
:resulttypmod -1
:resultcollid 0
:coercionformat 2
:location -1
}
A table rewrite means copying the existing, unaffected columns as-is. For the
column changing its data type, we first compute the COERCETODOMAIN node and
write the new value to the new table.
See ATRewriteTable->table_scan_getnextslot, The ExecEvalExpr below is
used to compute the new value of the changing column,
namely evaluating the COERCETODOMAIN node.
````
foreach(l, tab->newvals)
{
NewColumnValue *ex = lfirst(l);
if (ex->is_generated)
continue;
newslot->tts_values[ex->attnum - 1]
= ExecEvalExpr(ex->exprstate,
econtext,
&newslot->tts_isnull[ex->attnum - 1]);
}
````
For ALTER TABLE t ALTER COLUMN a SET DATA TYPE d1;
If we want to skip the table rewrite and do a table scan only,
We still need to use ExecEvalExpr() to evaluate the expression.
ExecEvalExpr() for COERCETODOMAIN may fail
(e.g `SELECT 1::d1`, where the value 1 cannot be cast to domain d1).
The last step of ExecInitExpr() is EEOP_DONE_RETURN, which means that when
ExecEvalExpr() evaluates an expression, if it does not
fail (No ``ereport(ERROR)`` happened), it will return a value.
see [1].
[1] https://git.postgresql.org/cgit/postgresql.git/commit/?id=8dd7c7cd0a2605d5301266a6b67a569d6a305106
> There are other things I don’t quite understand so will give it another pass once it’s been rebased.
>
This attached is more bullet-proof.
It can now cope with a domain over an array of another domain, where a
table scan
should be possible (as shown below).
+CREATE DOMAIN domain1 AS INT CHECK(VALUE > 1) NOT NULL;
+CREATE DOMAIN domain2 AS domain1 CHECK(VALUE > 1) NOT NULL;
+CREATE DOMAIN domain6 AS domain2[];
+ALTER TABLE t22 ALTER COLUMN col1 SET DATA TYPE domain6 USING col1;
| Attachment | Content-Type | Size |
|---|---|---|
| v4-0001-Skipping-table-rewrites-for-changing-column-types-in-some-cases.patch | text/x-patch | 17.3 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Nazir Bilal Yavuz | 2026-03-17 08:12:45 | Re: tupdesc: simplify assert in equalTupleDescs() |
| Previous Message | Chao Li | 2026-03-17 07:21:47 | tupdesc: simplify assert in equalTupleDescs() |