From b2c056c2e0d5d7ca52bcb859beb2416f8802cb21 Mon Sep 17 00:00:00 2001 From: jian he Date: Mon, 1 Sep 2025 13:47:47 +0800 Subject: [PATCH v7 4/4] table scan only when adding domain with volatile constraints When adding a new column, a table scan is sufficient to evaluate domains with volatile check constraints. example demo: CREATE DOMAIN domain8 as int check((value + random(min=>11::int, max=>11)) > 12); CREATE TABLE t3(a int); INSERT INTO t3 VALUES(1),(2); ALTER TABLE t3 ADD COLUMN f domain8 default 1; --error while coercing to domain ALTER TABLE t3 ADD COLUMN f domain8 default 20; --ok discussion: https://postgr.es/m/CACJufxE_+iZBR1i49k_AHigppPwLTJi6km8NOsC7FWvKdEmmXg@mail.gmail.com --- src/backend/commands/tablecmds.c | 72 ++++++++++++++++++---- src/test/regress/expected/fast_default.out | 27 +++++++- src/test/regress/sql/fast_default.sql | 15 ++++- 3 files changed, 98 insertions(+), 16 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index c58094a39c0..fcb378e79d5 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -230,6 +230,9 @@ typedef struct NewConstraint * are just copied from the old table during ATRewriteTable. Note that the * expr is an expression over *old* table values, except when is_generated * is true; then it is an expression over columns of the *new* tuple. + * + * If scan_only is true, it means in phase3, table scan is enough to evaulate expr. + * Currently, this is supported when expr is CoerceToDomain. */ typedef struct NewColumnValue { @@ -237,6 +240,8 @@ typedef struct NewColumnValue Expr *expr; /* expression to compute */ ExprState *exprstate; /* execution state */ bool is_generated; /* is it a GENERATED expression? */ + bool scan_only; /* use table scan to evaulate expression, + * useful only when table rewrite is false */ } NewColumnValue; /* @@ -6008,7 +6013,8 @@ 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); /* @@ -6118,7 +6124,9 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap) Relation newrel; TupleDesc oldTupDesc; TupleDesc newTupDesc; + TupleDesc oldTupDescTemp; bool needscan = false; + bool scan_only = false; List *notnull_attrs; List *notnull_virtual_attrs; int i; @@ -6135,6 +6143,7 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap) */ oldrel = table_open(tab->relid, NoLock); oldTupDesc = tab->oldDesc; + oldTupDescTemp = CreateTupleDescCopy(oldTupDesc); newTupDesc = RelationGetDescr(oldrel); /* includes all mods */ if (OidIsValid(OIDNewHeap)) @@ -6203,6 +6212,11 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap) /* expr already planned */ ex->exprstate = ExecInitExpr((Expr *) ex->expr, NULL); + if (ex->scan_only && !tab->rewrite && !scan_only) + { + needscan = true; + scan_only = true; + } } notnull_attrs = notnull_virtual_attrs = NIL; @@ -6431,6 +6445,43 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap) * new constraints etc. */ insertslot = oldslot; + + /* + * The tupdesc (newTupDesc) in oldslot already have the updated + * attribute changes. If we use it for below ExecEvalExpr, then + * CheckVarSlotCompatibility will fail. Therefore, we need to + * temporarily set oldslot's tts_tupleDescriptor equal to oldTupDesc. + * Essentially, what we're doing here is evaluating the + * CoerceToDomain node against the existing table slot. + */ + if (scan_only) + { + Datum values pg_attribute_unused(); + bool isnull pg_attribute_unused(); + + insertslot->tts_tupleDescriptor = oldTupDescTemp; + econtext->ecxt_scantuple = insertslot; + + foreach(l, tab->newvals) + { + NewColumnValue *ex = lfirst(l); + + if (!ex->scan_only) + continue; + + /* + * 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; + } + insertslot->tts_tupleDescriptor = newTupDesc; + } } /* Now check any constraints on the possibly-changed tuple */ @@ -7466,15 +7517,6 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel, has_domain_constraints = DomainHaveVolatileConstraints(attribute->atttypid, &has_volatile); - /* - * Adding column with volatile domain constraint requires table rewrite - */ - if (has_volatile) - { - Assert(has_domain_constraints); - tab->rewrite |= AT_REWRITE_DEFAULT_VAL; - } - /* Build CoerceToDomain(NULL) expression if needed */ if (!defval && has_domain_constraints) { @@ -7511,6 +7553,13 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel, newval->expr = defval; newval->is_generated = (colDef->generated != '\0'); + /* + * If the domain has volatile constraints, table scan is enough to + * evaluate CoerceToDomain, no need table rewrite. + */ + if (has_volatile && IsA(defval, CoerceToDomain)) + newval->scan_only = true; + tab->newvals = lappend(tab->newvals, newval); /* @@ -7528,7 +7577,6 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel, */ if (rel->rd_rel->relkind == RELKIND_RELATION && !colDef->generated && - !has_volatile && !contain_volatile_functions((Node *) defval)) { EState *estate; @@ -7560,7 +7608,7 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel, } FreeExecutorState(estate); } - else + else if (!has_volatile) { /* * Failed to use missing mode. We have to do a table rewrite diff --git a/src/test/regress/expected/fast_default.out b/src/test/regress/expected/fast_default.out index aa522cf8bfa..136454a289d 100644 --- a/src/test/regress/expected/fast_default.out +++ b/src/test/regress/expected/fast_default.out @@ -351,11 +351,34 @@ ORDER BY attnum; --need table rewrite when we are applying domain volatile default expresion ALTER TABLE t3 ADD COLUMN e domain7; NOTICE: rewriting table t3 for reason 2 --- need table rewrite when new column is domain with volatile constraints. -ALTER TABLE t3 ADD COLUMN f domain8 default 14; +--No need table rewrite when we are applying domain constraint volatile expresion, +--but table scan is required. +ALTER TABLE t3 ADD COLUMN f domain8 default 1; --error while coercing to domain NOTICE: rewriting table t3 for reason 2 +ERROR: value for domain domain8 violates check constraint "domain8_check" +ALTER TABLE t3 ADD COLUMN f domain8 default 20; --ok +ALTER DOMAIN domain8 ADD CHECK(VALUE IS NOT NULL); +--table rewrite then error, because f1 default value (NULL), does not satisfied with domain constraint ALTER TABLE t3 ADD COLUMN f1 domain8; NOTICE: rewriting table t3 for reason 2 +ERROR: value for domain domain8 violates check constraint "domain8_check1" +SELECT f FROM t3; + f +---- + 20 + 20 +(2 rows) + +SELECT attnum, attname, atthasmissing, atthasdef, attmissingval +FROM pg_attribute +WHERE attnum > 0 AND attrelid = 't3'::regclass AND attisdropped is false +AND atthasmissing +ORDER BY attnum; + attnum | attname | atthasmissing | atthasdef | attmissingval +--------+---------+---------------+-----------+--------------- + 6 | f | t | t | {20} +(1 row) + DROP TABLE t2; DROP TABLE t3; DROP DOMAIN domain1; diff --git a/src/test/regress/sql/fast_default.sql b/src/test/regress/sql/fast_default.sql index 269a39edf77..178249de960 100644 --- a/src/test/regress/sql/fast_default.sql +++ b/src/test/regress/sql/fast_default.sql @@ -315,10 +315,21 @@ ORDER BY attnum; --need table rewrite when we are applying domain volatile default expresion ALTER TABLE t3 ADD COLUMN e domain7; --- need table rewrite when new column is domain with volatile constraints. -ALTER TABLE t3 ADD COLUMN f domain8 default 14; +--No need table rewrite when we are applying domain constraint volatile expresion, +--but table scan is required. +ALTER TABLE t3 ADD COLUMN f domain8 default 1; --error while coercing to domain +ALTER TABLE t3 ADD COLUMN f domain8 default 20; --ok +ALTER DOMAIN domain8 ADD CHECK(VALUE IS NOT NULL); +--table rewrite then error, because f1 default value (NULL), does not satisfied with domain constraint ALTER TABLE t3 ADD COLUMN f1 domain8; +SELECT f FROM t3; +SELECT attnum, attname, atthasmissing, atthasdef, attmissingval +FROM pg_attribute +WHERE attnum > 0 AND attrelid = 't3'::regclass AND attisdropped is false +AND atthasmissing +ORDER BY attnum; + DROP TABLE t2; DROP TABLE t3; DROP DOMAIN domain1; -- 2.34.1