Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit95f6506

Browse files
committed
Fix broken handling of domains in atthasmissing logic.
If a domain type has a default, adding a column of that type (withoutany explicit DEFAULT clause) failed to install the domain's defaultvalue in existing rows, instead leaving the new column null. Thisis unexpected, and it used to work correctly before v11. The causeis confusion in the atthasmissing mechanism about which default valueto install: we'd only consider installing an explicitly-specifieddefault, and then we'd decide that no table rewrite is needed.To fix, take the responsibility for filling attmissingval out ofStoreAttrDefault, and instead put it into ATExecAddColumn's existinglogic that derives the correct value to fill the new column with.Also, centralize the logic that determines the need fordefault-related table rewriting there, instead of spreading it overfour or five places.In the back branches, we'll leave the attmissingval-filling codein StoreAttrDefault even though it's now dead, for fear that someextension may be depending on that functionality to exist there.A separate HEAD-only patch will clean up the now-useless code.Reported-by: jian he <jian.universality@gmail.com>Author: jian he <jian.universality@gmail.com>Author: Tom Lane <tgl@sss.pgh.pa.us>Discussion:https://postgr.es/m/CACJufxHFssPvkP1we7WMhPD_1kwgbG52o=kQgL+TnVoX5LOyCQ@mail.gmail.comBackpatch-through: 13
1 parent99f8f3f commit95f6506

File tree

6 files changed

+235
-34
lines changed

6 files changed

+235
-34
lines changed

‎src/backend/catalog/heap.c

Lines changed: 56 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@
6969
#include"pgstat.h"
7070
#include"storage/lmgr.h"
7171
#include"storage/predicate.h"
72+
#include"utils/array.h"
7273
#include"utils/builtins.h"
7374
#include"utils/fmgroids.h"
7475
#include"utils/inval.h"
@@ -2010,6 +2011,60 @@ RelationClearMissing(Relation rel)
20102011
table_close(attr_rel,RowExclusiveLock);
20112012
}
20122013

2014+
/*
2015+
* StoreAttrMissingVal
2016+
*
2017+
* Set the missing value of a single attribute.
2018+
*/
2019+
void
2020+
StoreAttrMissingVal(Relationrel,AttrNumberattnum,Datummissingval)
2021+
{
2022+
DatumvaluesAtt[Natts_pg_attribute]= {0};
2023+
boolnullsAtt[Natts_pg_attribute]= {0};
2024+
boolreplacesAtt[Natts_pg_attribute]= {0};
2025+
Relationattrrel;
2026+
Form_pg_attributeattStruct;
2027+
HeapTupleatttup,
2028+
newtup;
2029+
2030+
/* This is only supported for plain tables */
2031+
Assert(rel->rd_rel->relkind==RELKIND_RELATION);
2032+
2033+
/* Fetch the pg_attribute row */
2034+
attrrel=table_open(AttributeRelationId,RowExclusiveLock);
2035+
2036+
atttup=SearchSysCache2(ATTNUM,
2037+
ObjectIdGetDatum(RelationGetRelid(rel)),
2038+
Int16GetDatum(attnum));
2039+
if (!HeapTupleIsValid(atttup))/* shouldn't happen */
2040+
elog(ERROR,"cache lookup failed for attribute %d of relation %u",
2041+
attnum,RelationGetRelid(rel));
2042+
attStruct= (Form_pg_attribute)GETSTRUCT(atttup);
2043+
2044+
/* Make a one-element array containing the value */
2045+
missingval=PointerGetDatum(construct_array(&missingval,
2046+
1,
2047+
attStruct->atttypid,
2048+
attStruct->attlen,
2049+
attStruct->attbyval,
2050+
attStruct->attalign));
2051+
2052+
/* Update the pg_attribute row */
2053+
valuesAtt[Anum_pg_attribute_atthasmissing-1]=BoolGetDatum(true);
2054+
replacesAtt[Anum_pg_attribute_atthasmissing-1]= true;
2055+
2056+
valuesAtt[Anum_pg_attribute_attmissingval-1]=missingval;
2057+
replacesAtt[Anum_pg_attribute_attmissingval-1]= true;
2058+
2059+
newtup=heap_modify_tuple(atttup,RelationGetDescr(attrrel),
2060+
valuesAtt,nullsAtt,replacesAtt);
2061+
CatalogTupleUpdate(attrrel,&newtup->t_self,newtup);
2062+
2063+
/* clean up */
2064+
ReleaseSysCache(atttup);
2065+
table_close(attrrel,RowExclusiveLock);
2066+
}
2067+
20132068
/*
20142069
* SetAttrMissing
20152070
*
@@ -2392,13 +2447,8 @@ AddRelationNewConstraints(Relation rel,
23922447
castNode(Const,expr)->constisnull))
23932448
continue;
23942449

2395-
/* If the DEFAULT is volatile we cannot use a missing value */
2396-
if (colDef->missingMode&&
2397-
contain_volatile_functions_after_planning((Expr*)expr))
2398-
colDef->missingMode= false;
2399-
24002450
defOid=StoreAttrDefault(rel,colDef->attnum,expr,is_internal,
2401-
colDef->missingMode);
2451+
false);
24022452

24032453
cooked= (CookedConstraint*)palloc(sizeof(CookedConstraint));
24042454
cooked->contype=CONSTR_DEFAULT;

‎src/backend/catalog/pg_attrdef.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,12 @@ StoreAttrDefault(Relation rel, AttrNumber attnum,
120120
valuesAtt[Anum_pg_attribute_atthasdef-1]= true;
121121
replacesAtt[Anum_pg_attribute_atthasdef-1]= true;
122122

123+
/*
124+
* Note: this code is dead so far as core Postgres is concerned,
125+
* because no caller passes add_column_mode = true anymore. We keep
126+
* it in back branches on the slight chance that some extension is
127+
* depending on it.
128+
*/
123129
if (rel->rd_rel->relkind==RELKIND_RELATION&&add_column_mode&&
124130
!attgenerated)
125131
{

‎src/backend/commands/tablecmds.c

Lines changed: 60 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -7321,14 +7321,7 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
73217321
rawEnt = (RawColumnDefault *) palloc(sizeof(RawColumnDefault));
73227322
rawEnt->attnum = attribute->attnum;
73237323
rawEnt->raw_default = copyObject(colDef->raw_default);
7324-
7325-
/*
7326-
* Attempt to skip a complete table rewrite by storing the specified
7327-
* DEFAULT value outside of the heap. This may be disabled inside
7328-
* AddRelationNewConstraints if the optimization cannot be applied.
7329-
*/
7330-
rawEnt->missingMode = (colDef->generated != ATTRIBUTE_GENERATED_STORED);
7331-
7324+
rawEnt->missingMode = false;/* XXX vestigial */
73327325
rawEnt->generated = colDef->generated;
73337326

73347327
/*
@@ -7340,13 +7333,6 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
73407333

73417334
/* Make the additional catalog changes visible */
73427335
CommandCounterIncrement();
7343-
7344-
/*
7345-
* Did the request for a missing value work? If not we'll have to do a
7346-
* rewrite
7347-
*/
7348-
if (!rawEnt->missingMode)
7349-
tab->rewrite |= AT_REWRITE_DEFAULT_VAL;
73507336
}
73517337

73527338
/*
@@ -7363,9 +7349,9 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
73637349
* rejects nulls. If there are any domain constraints then we construct
73647350
* an explicit NULL default value that will be passed through
73657351
* CoerceToDomain processing. (This is a tad inefficient, since it causes
7366-
* rewriting the table which we reallydon't have to do, butthe present
7367-
*design of domain processing doesn't offer any simple way of checking
7368-
* theconstraints more directly.)
7352+
* rewriting the table which we reallywouldn't have to do; butwe do it
7353+
*to preserve the historical behavior that such a failure will be raised
7354+
*only ifthetable currently contains some rows.)
73697355
*
73707356
* Note: we use build_column_default, and not just the cooked default
73717357
* returned by AddRelationNewConstraints, so that the right thing happens
@@ -7384,6 +7370,9 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
73847370
*/
73857371
if (RELKIND_HAS_STORAGE(relkind))
73867372
{
7373+
boolhas_domain_constraints;
7374+
boolhas_missing = false;
7375+
73877376
/*
73887377
* For an identity column, we can't use build_column_default(),
73897378
* because the sequence ownership isn't set yet. So do it manually.
@@ -7396,14 +7385,13 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
73967385
nve->typeId = attribute->atttypid;
73977386

73987387
defval = (Expr *) nve;
7399-
7400-
/* must do a rewrite for identity columns */
7401-
tab->rewrite |= AT_REWRITE_DEFAULT_VAL;
74027388
}
74037389
else
74047390
defval = (Expr *) build_column_default(rel, attribute->attnum);
74057391

7406-
if (!defval && DomainHasConstraints(attribute->atttypid))
7392+
/* Build CoerceToDomain(NULL) expression if needed */
7393+
has_domain_constraints = DomainHasConstraints(attribute->atttypid);
7394+
if (!defval && has_domain_constraints)
74077395
{
74087396
OidbaseTypeId;
74097397
int32baseTypeMod;
@@ -7429,18 +7417,63 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
74297417
{
74307418
NewColumnValue *newval;
74317419

7420+
/* Prepare defval for execution, either here or in Phase 3 */
7421+
defval = expression_planner(defval);
7422+
7423+
/* Add the new default to the newvals list */
74327424
newval = (NewColumnValue *) palloc0(sizeof(NewColumnValue));
74337425
newval->attnum = attribute->attnum;
7434-
newval->expr =expression_planner(defval);
7426+
newval->expr = defval;
74357427
newval->is_generated = (colDef->generated != '\0');
74367428

74377429
tab->newvals = lappend(tab->newvals, newval);
7438-
}
74397430

7440-
if (DomainHasConstraints(attribute->atttypid))
7441-
tab->rewrite |= AT_REWRITE_DEFAULT_VAL;
7431+
/*
7432+
* Attempt to skip a complete table rewrite by storing the
7433+
* specified DEFAULT value outside of the heap. This is only
7434+
* allowed for plain relations and non-generated columns, and the
7435+
* default expression can't be volatile (stable is OK). Note that
7436+
* contain_volatile_functions deems CoerceToDomain immutable, but
7437+
* here we consider that coercion to a domain with constraints is
7438+
* volatile; else it might fail even when the table is empty.
7439+
*/
7440+
if (rel->rd_rel->relkind == RELKIND_RELATION &&
7441+
!colDef->generated &&
7442+
!has_domain_constraints &&
7443+
!contain_volatile_functions((Node *) defval))
7444+
{
7445+
EState *estate;
7446+
ExprState *exprState;
7447+
Datummissingval;
7448+
boolmissingIsNull;
7449+
7450+
/* Evaluate the default expression */
7451+
estate = CreateExecutorState();
7452+
exprState = ExecPrepareExpr(defval, estate);
7453+
missingval = ExecEvalExpr(exprState,
7454+
GetPerTupleExprContext(estate),
7455+
&missingIsNull);
7456+
/* If it turns out NULL, nothing to do; else store it */
7457+
if (!missingIsNull)
7458+
{
7459+
StoreAttrMissingVal(rel, attribute->attnum, missingval);
7460+
has_missing = true;
7461+
}
7462+
FreeExecutorState(estate);
7463+
}
7464+
else
7465+
{
7466+
/*
7467+
* Failed to use missing mode. We have to do a table rewrite
7468+
* to install the value --- unless it's a virtual generated
7469+
* column.
7470+
*/
7471+
if (colDef->generated != ATTRIBUTE_GENERATED_VIRTUAL)
7472+
tab->rewrite |= AT_REWRITE_DEFAULT_VAL;
7473+
}
7474+
}
74427475

7443-
if (!TupleDescAttr(rel->rd_att, attribute->attnum - 1)->atthasmissing)
7476+
if (!has_missing)
74447477
{
74457478
/*
74467479
* If the new column is NOT NULL, and there is no missing value,

‎src/include/catalog/heap.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ typedef struct RawColumnDefault
2929
{
3030
AttrNumberattnum;/* attribute to attach default to */
3131
Node*raw_default;/* default value (untransformed parse tree) */
32-
boolmissingMode;/*true if part of add column processing */
32+
boolmissingMode;/*obsolete, no longer used */
3333
chargenerated;/* attgenerated setting */
3434
}RawColumnDefault;
3535

@@ -121,6 +121,9 @@ extern List *AddRelationNotNullConstraints(Relation rel,
121121
List*old_notnulls);
122122

123123
externvoidRelationClearMissing(Relationrel);
124+
125+
externvoidStoreAttrMissingVal(Relationrel,AttrNumberattnum,
126+
Datummissingval);
124127
externvoidSetAttrMissing(Oidrelid,char*attname,char*value);
125128

126129
externNode*cookDefault(ParseState*pstate,

‎src/test/regress/expected/fast_default.out

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,71 @@ SELECT comp();
257257
(1 row)
258258

259259
DROP TABLE T;
260+
-- Test domains with default value for table rewrite.
261+
CREATE DOMAIN domain1 AS int DEFAULT 11; -- constant
262+
CREATE DOMAIN domain2 AS int DEFAULT random(min=>10, max=>100); -- volatile
263+
CREATE DOMAIN domain3 AS text DEFAULT foo(4); -- stable
264+
CREATE DOMAIN domain4 AS text[]
265+
DEFAULT ('{"This", "is", "' || foo(4) || '","the", "real", "world"}')::TEXT[];
266+
CREATE TABLE t2 (a domain1);
267+
INSERT INTO t2 VALUES (1),(2);
268+
-- no table rewrite
269+
ALTER TABLE t2 ADD COLUMN b domain1 default 3;
270+
SELECT attnum, attname, atthasmissing, atthasdef, attmissingval
271+
FROM pg_attribute
272+
WHERE attnum > 0 AND attrelid = 't2'::regclass
273+
ORDER BY attnum;
274+
attnum | attname | atthasmissing | atthasdef | attmissingval
275+
--------+---------+---------------+-----------+---------------
276+
1 | a | f | f |
277+
2 | b | t | t | {3}
278+
(2 rows)
279+
280+
-- table rewrite should happen
281+
ALTER TABLE t2 ADD COLUMN c domain3 default left(random()::text,3);
282+
NOTICE: rewriting table t2 for reason 2
283+
-- no table rewrite
284+
ALTER TABLE t2 ADD COLUMN d domain4;
285+
SELECT attnum, attname, atthasmissing, atthasdef, attmissingval
286+
FROM pg_attribute
287+
WHERE attnum > 0 AND attrelid = 't2'::regclass
288+
ORDER BY attnum;
289+
attnum | attname | atthasmissing | atthasdef | attmissingval
290+
--------+---------+---------------+-----------+-----------------------------------
291+
1 | a | f | f |
292+
2 | b | f | t |
293+
3 | c | f | t |
294+
4 | d | t | f | {"{This,is,abcd,the,real,world}"}
295+
(4 rows)
296+
297+
-- table rewrite should happen
298+
ALTER TABLE t2 ADD COLUMN e domain2;
299+
NOTICE: rewriting table t2 for reason 2
300+
SELECT attnum, attname, atthasmissing, atthasdef, attmissingval
301+
FROM pg_attribute
302+
WHERE attnum > 0 AND attrelid = 't2'::regclass
303+
ORDER BY attnum;
304+
attnum | attname | atthasmissing | atthasdef | attmissingval
305+
--------+---------+---------------+-----------+---------------
306+
1 | a | f | f |
307+
2 | b | f | t |
308+
3 | c | f | t |
309+
4 | d | f | f |
310+
5 | e | f | f |
311+
(5 rows)
312+
313+
SELECT a, b, length(c) = 3 as c_ok, d, e >= 10 as e_ok FROM t2;
314+
a | b | c_ok | d | e_ok
315+
---+---+------+-------------------------------+------
316+
1 | 3 | t | {This,is,abcd,the,real,world} | t
317+
2 | 3 | t | {This,is,abcd,the,real,world} | t
318+
(2 rows)
319+
320+
DROP TABLE t2;
321+
DROP DOMAIN domain1;
322+
DROP DOMAIN domain2;
323+
DROP DOMAIN domain3;
324+
DROP DOMAIN domain4;
260325
DROP FUNCTION foo(INT);
261326
-- Fall back to full rewrite for volatile expressions
262327
CREATE TABLE T(pk INT NOT NULL PRIMARY KEY);

‎src/test/regress/sql/fast_default.sql

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,50 @@ SELECT comp();
248248

249249
DROPTABLE T;
250250

251+
-- Test domains with default value for table rewrite.
252+
CREATEDOMAINdomain1ASint DEFAULT11;-- constant
253+
CREATEDOMAINdomain2ASint DEFAULT random(min=>10, max=>100);-- volatile
254+
CREATEDOMAINdomain3AStext DEFAULT foo(4);-- stable
255+
CREATEDOMAINdomain4AStext[]
256+
DEFAULT ('{"This", "is", "'|| foo(4)||'","the", "real", "world"}')::TEXT[];
257+
258+
CREATETABLEt2 (a domain1);
259+
INSERT INTO t2VALUES (1),(2);
260+
261+
-- no table rewrite
262+
ALTERTABLE t2 ADD COLUMN b domain1 default3;
263+
264+
SELECT attnum, attname, atthasmissing, atthasdef, attmissingval
265+
FROM pg_attribute
266+
WHERE attnum>0AND attrelid='t2'::regclass
267+
ORDER BY attnum;
268+
269+
-- table rewrite should happen
270+
ALTERTABLE t2 ADD COLUMN c domain3 default left(random()::text,3);
271+
272+
-- no table rewrite
273+
ALTERTABLE t2 ADD COLUMN d domain4;
274+
275+
SELECT attnum, attname, atthasmissing, atthasdef, attmissingval
276+
FROM pg_attribute
277+
WHERE attnum>0AND attrelid='t2'::regclass
278+
ORDER BY attnum;
279+
280+
-- table rewrite should happen
281+
ALTERTABLE t2 ADD COLUMN e domain2;
282+
283+
SELECT attnum, attname, atthasmissing, atthasdef, attmissingval
284+
FROM pg_attribute
285+
WHERE attnum>0AND attrelid='t2'::regclass
286+
ORDER BY attnum;
287+
288+
SELECT a, b, length(c)=3as c_ok, d, e>=10as e_okFROM t2;
289+
290+
DROPTABLE t2;
291+
DROPDOMAIN domain1;
292+
DROPDOMAIN domain2;
293+
DROPDOMAIN domain3;
294+
DROPDOMAIN domain4;
251295
DROPFUNCTION foo(INT);
252296

253297
-- Fall back to full rewrite for volatile expressions

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp