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

Commitd6dd2a0

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 parentd69c781 commitd6dd2a0

File tree

6 files changed

+233
-34
lines changed

6 files changed

+233
-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"
@@ -2003,6 +2004,60 @@ RelationClearMissing(Relation rel)
20032004
table_close(attr_rel,RowExclusiveLock);
20042005
}
20052006

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

2333-
/* If the DEFAULT is volatile we cannot use a missing value */
2334-
if (colDef->missingMode&&
2335-
contain_volatile_functions_after_planning((Expr*)expr))
2336-
colDef->missingMode= false;
2337-
23382388
defOid=StoreAttrDefault(rel,colDef->attnum,expr,is_internal,
2339-
colDef->missingMode);
2389+
false);
23402390

23412391
cooked= (CookedConstraint*)palloc(sizeof(CookedConstraint));
23422392
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: 58 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -7209,14 +7209,7 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
72097209
rawEnt = (RawColumnDefault *) palloc(sizeof(RawColumnDefault));
72107210
rawEnt->attnum = attribute->attnum;
72117211
rawEnt->raw_default = copyObject(colDef->raw_default);
7212-
7213-
/*
7214-
* Attempt to skip a complete table rewrite by storing the specified
7215-
* DEFAULT value outside of the heap. This may be disabled inside
7216-
* AddRelationNewConstraints if the optimization cannot be applied.
7217-
*/
7218-
rawEnt->missingMode = (!colDef->generated);
7219-
7212+
rawEnt->missingMode = false;/* XXX vestigial */
72207213
rawEnt->generated = colDef->generated;
72217214

72227215
/*
@@ -7228,13 +7221,6 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
72287221

72297222
/* Make the additional catalog changes visible */
72307223
CommandCounterIncrement();
7231-
7232-
/*
7233-
* Did the request for a missing value work? If not we'll have to do a
7234-
* rewrite
7235-
*/
7236-
if (!rawEnt->missingMode)
7237-
tab->rewrite |= AT_REWRITE_DEFAULT_VAL;
72387224
}
72397225

72407226
/*
@@ -7251,9 +7237,9 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
72517237
* rejects nulls. If there are any domain constraints then we construct
72527238
* an explicit NULL default value that will be passed through
72537239
* CoerceToDomain processing. (This is a tad inefficient, since it causes
7254-
* rewriting the table which we reallydon't have to do, butthe present
7255-
*design of domain processing doesn't offer any simple way of checking
7256-
* theconstraints more directly.)
7240+
* rewriting the table which we reallywouldn't have to do; butwe do it
7241+
*to preserve the historical behavior that such a failure will be raised
7242+
*only ifthetable currently contains some rows.)
72577243
*
72587244
* Note: we use build_column_default, and not just the cooked default
72597245
* returned by AddRelationNewConstraints, so that the right thing happens
@@ -7272,6 +7258,9 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
72727258
*/
72737259
if (RELKIND_HAS_STORAGE(relkind))
72747260
{
7261+
boolhas_domain_constraints;
7262+
boolhas_missing = false;
7263+
72757264
/*
72767265
* For an identity column, we can't use build_column_default(),
72777266
* because the sequence ownership isn't set yet. So do it manually.
@@ -7284,14 +7273,13 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
72847273
nve->typeId = attribute->atttypid;
72857274

72867275
defval = (Expr *) nve;
7287-
7288-
/* must do a rewrite for identity columns */
7289-
tab->rewrite |= AT_REWRITE_DEFAULT_VAL;
72907276
}
72917277
else
72927278
defval = (Expr *) build_column_default(rel, attribute->attnum);
72937279

7294-
if (!defval && DomainHasConstraints(attribute->atttypid))
7280+
/* Build CoerceToDomain(NULL) expression if needed */
7281+
has_domain_constraints = DomainHasConstraints(attribute->atttypid);
7282+
if (!defval && has_domain_constraints)
72957283
{
72967284
OidbaseTypeId;
72977285
int32baseTypeMod;
@@ -7317,18 +7305,61 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
73177305
{
73187306
NewColumnValue *newval;
73197307

7308+
/* Prepare defval for execution, either here or in Phase 3 */
7309+
defval = expression_planner(defval);
7310+
7311+
/* Add the new default to the newvals list */
73207312
newval = (NewColumnValue *) palloc0(sizeof(NewColumnValue));
73217313
newval->attnum = attribute->attnum;
7322-
newval->expr =expression_planner(defval);
7314+
newval->expr = defval;
73237315
newval->is_generated = (colDef->generated != '\0');
73247316

73257317
tab->newvals = lappend(tab->newvals, newval);
7326-
}
73277318

7328-
if (DomainHasConstraints(attribute->atttypid))
7329-
tab->rewrite |= AT_REWRITE_DEFAULT_VAL;
7319+
/*
7320+
* Attempt to skip a complete table rewrite by storing the
7321+
* specified DEFAULT value outside of the heap. This is only
7322+
* allowed for plain relations and non-generated columns, and the
7323+
* default expression can't be volatile (stable is OK). Note that
7324+
* contain_volatile_functions deems CoerceToDomain immutable, but
7325+
* here we consider that coercion to a domain with constraints is
7326+
* volatile; else it might fail even when the table is empty.
7327+
*/
7328+
if (rel->rd_rel->relkind == RELKIND_RELATION &&
7329+
!colDef->generated &&
7330+
!has_domain_constraints &&
7331+
!contain_volatile_functions((Node *) defval))
7332+
{
7333+
EState *estate;
7334+
ExprState *exprState;
7335+
Datummissingval;
7336+
boolmissingIsNull;
7337+
7338+
/* Evaluate the default expression */
7339+
estate = CreateExecutorState();
7340+
exprState = ExecPrepareExpr(defval, estate);
7341+
missingval = ExecEvalExpr(exprState,
7342+
GetPerTupleExprContext(estate),
7343+
&missingIsNull);
7344+
/* If it turns out NULL, nothing to do; else store it */
7345+
if (!missingIsNull)
7346+
{
7347+
StoreAttrMissingVal(rel, attribute->attnum, missingval);
7348+
has_missing = true;
7349+
}
7350+
FreeExecutorState(estate);
7351+
}
7352+
else
7353+
{
7354+
/*
7355+
* Failed to use missing mode. We have to do a table rewrite
7356+
* to install the value.
7357+
*/
7358+
tab->rewrite |= AT_REWRITE_DEFAULT_VAL;
7359+
}
7360+
}
73307361

7331-
if (!TupleDescAttr(rel->rd_att, attribute->attnum - 1)->atthasmissing)
7362+
if (!has_missing)
73327363
{
73337364
/*
73347365
* 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
@@ -28,7 +28,7 @@ typedef struct RawColumnDefault
2828
{
2929
AttrNumberattnum;/* attribute to attach default to */
3030
Node*raw_default;/* default value (untransformed parse tree) */
31-
boolmissingMode;/*true if part of add column processing */
31+
boolmissingMode;/*obsolete, no longer used */
3232
chargenerated;/* attgenerated setting */
3333
}RawColumnDefault;
3434

@@ -115,6 +115,9 @@ extern List *AddRelationNewConstraints(Relation rel,
115115
constchar*queryString);
116116

117117
externvoidRelationClearMissing(Relationrel);
118+
119+
externvoidStoreAttrMissingVal(Relationrel,AttrNumberattnum,
120+
Datummissingval);
118121
externvoidSetAttrMissing(Oidrelid,char*attname,char*value);
119122

120123
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
@@ -245,6 +245,71 @@ SELECT comp();
245245
(1 row)
246246

247247
DROP TABLE T;
248+
-- Test domains with default value for table rewrite.
249+
CREATE DOMAIN domain1 AS int DEFAULT 11; -- constant
250+
CREATE DOMAIN domain2 AS int DEFAULT random(min=>10, max=>100); -- volatile
251+
CREATE DOMAIN domain3 AS text DEFAULT foo(4); -- stable
252+
CREATE DOMAIN domain4 AS text[]
253+
DEFAULT ('{"This", "is", "' || foo(4) || '","the", "real", "world"}')::TEXT[];
254+
CREATE TABLE t2 (a domain1);
255+
INSERT INTO t2 VALUES (1),(2);
256+
-- no table rewrite
257+
ALTER TABLE t2 ADD COLUMN b domain1 default 3;
258+
SELECT attnum, attname, atthasmissing, atthasdef, attmissingval
259+
FROM pg_attribute
260+
WHERE attnum > 0 AND attrelid = 't2'::regclass
261+
ORDER BY attnum;
262+
attnum | attname | atthasmissing | atthasdef | attmissingval
263+
--------+---------+---------------+-----------+---------------
264+
1 | a | f | f |
265+
2 | b | t | t | {3}
266+
(2 rows)
267+
268+
-- table rewrite should happen
269+
ALTER TABLE t2 ADD COLUMN c domain3 default left(random()::text,3);
270+
NOTICE: rewriting table t2 for reason 2
271+
-- no table rewrite
272+
ALTER TABLE t2 ADD COLUMN d domain4;
273+
SELECT attnum, attname, atthasmissing, atthasdef, attmissingval
274+
FROM pg_attribute
275+
WHERE attnum > 0 AND attrelid = 't2'::regclass
276+
ORDER BY attnum;
277+
attnum | attname | atthasmissing | atthasdef | attmissingval
278+
--------+---------+---------------+-----------+-----------------------------------
279+
1 | a | f | f |
280+
2 | b | f | t |
281+
3 | c | f | t |
282+
4 | d | t | f | {"{This,is,abcd,the,real,world}"}
283+
(4 rows)
284+
285+
-- table rewrite should happen
286+
ALTER TABLE t2 ADD COLUMN e domain2;
287+
NOTICE: rewriting table t2 for reason 2
288+
SELECT attnum, attname, atthasmissing, atthasdef, attmissingval
289+
FROM pg_attribute
290+
WHERE attnum > 0 AND attrelid = 't2'::regclass
291+
ORDER BY attnum;
292+
attnum | attname | atthasmissing | atthasdef | attmissingval
293+
--------+---------+---------------+-----------+---------------
294+
1 | a | f | f |
295+
2 | b | f | t |
296+
3 | c | f | t |
297+
4 | d | f | f |
298+
5 | e | f | f |
299+
(5 rows)
300+
301+
SELECT a, b, length(c) = 3 as c_ok, d, e >= 10 as e_ok FROM t2;
302+
a | b | c_ok | d | e_ok
303+
---+---+------+-------------------------------+------
304+
1 | 3 | t | {This,is,abcd,the,real,world} | t
305+
2 | 3 | t | {This,is,abcd,the,real,world} | t
306+
(2 rows)
307+
308+
DROP TABLE t2;
309+
DROP DOMAIN domain1;
310+
DROP DOMAIN domain2;
311+
DROP DOMAIN domain3;
312+
DROP DOMAIN domain4;
248313
DROP FUNCTION foo(INT);
249314
-- Fall back to full rewrite for volatile expressions
250315
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
@@ -237,6 +237,50 @@ SELECT comp();
237237

238238
DROPTABLE T;
239239

240+
-- Test domains with default value for table rewrite.
241+
CREATEDOMAINdomain1ASint DEFAULT11;-- constant
242+
CREATEDOMAINdomain2ASint DEFAULT random(min=>10, max=>100);-- volatile
243+
CREATEDOMAINdomain3AStext DEFAULT foo(4);-- stable
244+
CREATEDOMAINdomain4AStext[]
245+
DEFAULT ('{"This", "is", "'|| foo(4)||'","the", "real", "world"}')::TEXT[];
246+
247+
CREATETABLEt2 (a domain1);
248+
INSERT INTO t2VALUES (1),(2);
249+
250+
-- no table rewrite
251+
ALTERTABLE t2 ADD COLUMN b domain1 default3;
252+
253+
SELECT attnum, attname, atthasmissing, atthasdef, attmissingval
254+
FROM pg_attribute
255+
WHERE attnum>0AND attrelid='t2'::regclass
256+
ORDER BY attnum;
257+
258+
-- table rewrite should happen
259+
ALTERTABLE t2 ADD COLUMN c domain3 default left(random()::text,3);
260+
261+
-- no table rewrite
262+
ALTERTABLE t2 ADD COLUMN d domain4;
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 e domain2;
271+
272+
SELECT attnum, attname, atthasmissing, atthasdef, attmissingval
273+
FROM pg_attribute
274+
WHERE attnum>0AND attrelid='t2'::regclass
275+
ORDER BY attnum;
276+
277+
SELECT a, b, length(c)=3as c_ok, d, e>=10as e_okFROM t2;
278+
279+
DROPTABLE t2;
280+
DROPDOMAIN domain1;
281+
DROPDOMAIN domain2;
282+
DROPDOMAIN domain3;
283+
DROPDOMAIN domain4;
240284
DROPFUNCTION foo(INT);
241285

242286
-- Fall back to full rewrite for volatile expressions

0 commit comments

Comments
 (0)

[8]ページ先頭

©2009-2025 Movatter.jp