On 17 Mar 2026 at 09:06 +0100, jian he <[email protected]>, wrote:
> This attached is more bullet-proof.
My previous question was more of syntax question, and it has been addressed in 
the latest patch.

Questions/ comments:

The commitfest entry has two other reviewers already, but I haven’t seen any 
emails from them? If you are still reviewing Yogesh or Aditya, maybe let us 
know, or else might be best to remove them from the entry so it’s visible 
you’re waiting for reviewers.

-----------------------

+ or an unconstrained domain over the new type, or domain over new type has no
+ volatile constraint, a table rewrite is not needed.
+ However, indexes will still be rebuilt unless the system

Why do we need to care about volatility in this patch? Wouldn’t things work 
with skipping the rewrite then checking the volatile checks? A rewrite doesn’t 
make things more deterministic right?

------------------------

+DROP DOMAIN domain1, domain2, domain3, domain4, domain5;
+ERROR: cannot drop desired object(s) because other objects depend on them
+DETAIL: type domain6 depends on type domain2[]
+HINT: Use DROP ... CASCADE to drop the dependent objects too.

+ERROR: cannot drop schema fast_default because other objects depend on it
+DETAIL: type domain1 depends on schema fast_default
+type domain2 depends on schema fast_default
+type domain3 depends on schema fast_default
+type domain4 depends on schema fast_default
+type domain5 depends on schema fast_default
+type domain6 depends on schema fast_default
+HINT: Use DROP ... CASCADE to drop the dependent objects too.

Unnecessary noise in the test output?

-----------------------

Add a test like this:

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

So something is wrong with constant values.

-----------------------

Minor spelling/grammar:
- Typo: igrnore → ignore (line ~6552)
- Grammar issues in comments and the commit message ("We can just using table 
scan", "ensure exists content is compatibility", "the new domain type all 
constraint are non-volatile").

-----------------------

This is very much a non-exhaustive review as I in all honestly understand maybe 
like 20% of this code. But at least it’s a start.

Reply via email to