Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 35c8dd9

Browse files
committed
Simplify some logic around setting pg_attribute.atthasdef.
DefineRelation was of the opinion that it could usefully pre-fill atthasdef flags to eliminate work for StoreAttrDefault. This is not the case, however: the tupledesc that it's filling is not the one that InsertPgAttributeTuples will work from. The tupledesc used there is made by RelationBuildLocalRelation, which deliberately doesn't copy atthasdef. Moreover, if this did happen as the code thinks, it would be wrong for the case of plain "DEFAULT NULL" clauses, since we detect and ignore simple-null-Const defaults later on. Hence, remove the useless code. It also emerges that it's not really worth a special-case path in StoreAttrDefault() for atthasdef already being set, because as far as we can see that never happens: cases where an existing default gets updated always do RemoveAttrDefault first, so as to clean up possibly-no-longer-correct dependency entries. If it were the case the code would still work, anyway. Also remove a nearby comment made moot by 5eaa0e9. Author: jian he <jian.universality@gmail.com> Discussion: https://postgr.es/m/CACJufxHFssPvkP1we7WMhPD_1kwgbG52o=kQgL+TnVoX5LOyCQ@mail.gmail.com
1 parent 4528768 commit 35c8dd9

File tree

2 files changed

+10
-24
lines changed

2 files changed

+10
-24
lines changed

src/backend/catalog/pg_attrdef.c

+9-11
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,9 @@ StoreAttrDefault(Relation rel, AttrNumber attnum,
4343
Relation attrrel;
4444
HeapTuple atttup;
4545
Form_pg_attribute attStruct;
46+
Datum valuesAtt[Natts_pg_attribute] = {0};
47+
bool nullsAtt[Natts_pg_attribute] = {0};
48+
bool replacesAtt[Natts_pg_attribute] = {0};
4649
char attgenerated;
4750
Oid attrdefOid;
4851
ObjectAddress colobject,
@@ -92,20 +95,15 @@ StoreAttrDefault(Relation rel, AttrNumber attnum,
9295
attnum, RelationGetRelid(rel));
9396
attStruct = (Form_pg_attribute) GETSTRUCT(atttup);
9497
attgenerated = attStruct->attgenerated;
95-
if (!attStruct->atthasdef)
96-
{
97-
Datum valuesAtt[Natts_pg_attribute] = {0};
98-
bool nullsAtt[Natts_pg_attribute] = {0};
99-
bool replacesAtt[Natts_pg_attribute] = {0};
10098

101-
valuesAtt[Anum_pg_attribute_atthasdef - 1] = BoolGetDatum(true);
102-
replacesAtt[Anum_pg_attribute_atthasdef - 1] = true;
99+
valuesAtt[Anum_pg_attribute_atthasdef - 1] = BoolGetDatum(true);
100+
replacesAtt[Anum_pg_attribute_atthasdef - 1] = true;
103101

104-
atttup = heap_modify_tuple(atttup, RelationGetDescr(attrrel),
105-
valuesAtt, nullsAtt, replacesAtt);
102+
atttup = heap_modify_tuple(atttup, RelationGetDescr(attrrel),
103+
valuesAtt, nullsAtt, replacesAtt);
104+
105+
CatalogTupleUpdate(attrrel, &atttup->t_self, atttup);
106106

107-
CatalogTupleUpdate(attrrel, &atttup->t_self, atttup);
108-
}
109107
table_close(attrrel, RowExclusiveLock);
110108
heap_freetuple(atttup);
111109

src/backend/commands/tablecmds.c

+1-13
Original file line numberDiff line numberDiff line change
@@ -941,10 +941,6 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
941941
* while raw defaults go into a list of RawColumnDefault structs that will
942942
* be processed by AddRelationNewConstraints. (We can't deal with raw
943943
* expressions until we can do transformExpr.)
944-
*
945-
* We can set the atthasdef flags now in the tuple descriptor; this just
946-
* saves StoreAttrDefault from having to do an immediate update of the
947-
* pg_attribute rows.
948944
*/
949945
rawDefaults = NIL;
950946
cookedDefaults = NIL;
@@ -953,11 +949,8 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
953949
foreach(listptr, stmt->tableElts)
954950
{
955951
ColumnDef *colDef = lfirst(listptr);
956-
Form_pg_attribute attr;
957952

958953
attnum++;
959-
attr = TupleDescAttr(descriptor, attnum - 1);
960-
961954
if (colDef->raw_default != NULL)
962955
{
963956
RawColumnDefault *rawEnt;
@@ -969,7 +962,6 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
969962
rawEnt->raw_default = colDef->raw_default;
970963
rawEnt->generated = colDef->generated;
971964
rawDefaults = lappend(rawDefaults, rawEnt);
972-
attr->atthasdef = true;
973965
}
974966
else if (colDef->cooked_default != NULL)
975967
{
@@ -987,10 +979,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
987979
cooked->inhcount = 0; /* ditto */
988980
cooked->is_no_inherit = false;
989981
cookedDefaults = lappend(cookedDefaults, cooked);
990-
attr->atthasdef = true;
991982
}
992-
993-
populate_compact_attribute(descriptor, attnum - 1);
994983
}
995984

996985
/*
@@ -7363,8 +7352,7 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
73637352
* and the later subcommands had been issued in new ALTER TABLE commands.
73647353
*
73657354
* We can skip this entirely for relations without storage, since Phase 3
7366-
* is certainly not going to touch them. System attributes don't have
7367-
* interesting defaults, either.
7355+
* is certainly not going to touch them.
73687356
*/
73697357
if (RELKIND_HAS_STORAGE(relkind))
73707358
{

0 commit comments

Comments
 (0)