From 4528768d98f83070d625207dcb526982fb18f1ce Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 3 Mar 2025 13:09:20 -0500 Subject: [PATCH] Remove now-dead code in StoreAttrDefault(). StoreAttrDefault() is no longer responsible for filling attmissingval, so remove the code for that. Get rid of RawColumnDefault.missingMode, too, as we no longer need that to pass information around. While here, clean up some sloppy coding in StoreAttrDefault(), such as failure to use XXXGetDatum macros. These aren't bugs but they're not good code either. Reported-by: jian he Author: jian he Author: Tom Lane Discussion: https://postgr.es/m/CACJufxHFssPvkP1we7WMhPD_1kwgbG52o=kQgL+TnVoX5LOyCQ@mail.gmail.com --- src/backend/catalog/heap.c | 5 +-- src/backend/catalog/pg_attrdef.c | 77 +++----------------------------- src/backend/commands/tablecmds.c | 8 +--- src/include/catalog/heap.h | 1 - src/include/catalog/pg_attrdef.h | 3 +- 5 files changed, 11 insertions(+), 83 deletions(-) diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index f7f36837d76..bd3554c0bfd 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -2320,7 +2320,7 @@ StoreConstraints(Relation rel, List *cooked_constraints, bool is_internal) { case CONSTR_DEFAULT: con->conoid = StoreAttrDefault(rel, con->attnum, con->expr, - is_internal, false); + is_internal); break; case CONSTR_CHECK: con->conoid = @@ -2447,8 +2447,7 @@ AddRelationNewConstraints(Relation rel, castNode(Const, expr)->constisnull)) continue; - defOid = StoreAttrDefault(rel, colDef->attnum, expr, is_internal, - false); + defOid = StoreAttrDefault(rel, colDef->attnum, expr, is_internal); cooked = (CookedConstraint *) palloc(sizeof(CookedConstraint)); cooked->contype = CONSTR_DEFAULT; diff --git a/src/backend/catalog/pg_attrdef.c b/src/backend/catalog/pg_attrdef.c index a58815a5134..f2773682dfb 100644 --- a/src/backend/catalog/pg_attrdef.c +++ b/src/backend/catalog/pg_attrdef.c @@ -14,17 +14,12 @@ */ #include "postgres.h" -#include "access/genam.h" #include "access/relation.h" #include "access/table.h" -#include "catalog/catalog.h" #include "catalog/dependency.h" #include "catalog/indexing.h" #include "catalog/objectaccess.h" #include "catalog/pg_attrdef.h" -#include "executor/executor.h" -#include "optimizer/optimizer.h" -#include "utils/array.h" #include "utils/builtins.h" #include "utils/fmgroids.h" #include "utils/rel.h" @@ -35,22 +30,16 @@ * Store a default expression for column attnum of relation rel. * * Returns the OID of the new pg_attrdef tuple. - * - * add_column_mode must be true if we are storing the default for a new - * attribute, and false if it's for an already existing attribute. The reason - * for this is that the missing value must never be updated after it is set, - * which can only be when a column is added to the table. Otherwise we would - * in effect be changing existing tuples. */ Oid StoreAttrDefault(Relation rel, AttrNumber attnum, - Node *expr, bool is_internal, bool add_column_mode) + Node *expr, bool is_internal) { char *adbin; Relation adrel; HeapTuple tuple; - Datum values[4]; - static bool nulls[4] = {false, false, false, false}; + Datum values[Natts_pg_attrdef]; + static bool nulls[Natts_pg_attrdef] = {false, false, false, false}; Relation attrrel; HeapTuple atttup; Form_pg_attribute attStruct; @@ -72,8 +61,8 @@ StoreAttrDefault(Relation rel, AttrNumber attnum, attrdefOid = GetNewOidWithIndex(adrel, AttrDefaultOidIndexId, Anum_pg_attrdef_oid); values[Anum_pg_attrdef_oid - 1] = ObjectIdGetDatum(attrdefOid); - values[Anum_pg_attrdef_adrelid - 1] = RelationGetRelid(rel); - values[Anum_pg_attrdef_adnum - 1] = attnum; + values[Anum_pg_attrdef_adrelid - 1] = ObjectIdGetDatum(RelationGetRelid(rel)); + values[Anum_pg_attrdef_adnum - 1] = Int16GetDatum(attnum); values[Anum_pg_attrdef_adbin - 1] = CStringGetTextDatum(adbin); tuple = heap_form_tuple(adrel->rd_att, values, nulls); @@ -105,71 +94,17 @@ StoreAttrDefault(Relation rel, AttrNumber attnum, attgenerated = attStruct->attgenerated; if (!attStruct->atthasdef) { - Form_pg_attribute defAttStruct; - - ExprState *exprState; - Expr *expr2 = (Expr *) expr; - EState *estate = NULL; - ExprContext *econtext; Datum valuesAtt[Natts_pg_attribute] = {0}; bool nullsAtt[Natts_pg_attribute] = {0}; bool replacesAtt[Natts_pg_attribute] = {0}; - Datum missingval = (Datum) 0; - bool missingIsNull = true; - valuesAtt[Anum_pg_attribute_atthasdef - 1] = true; + valuesAtt[Anum_pg_attribute_atthasdef - 1] = BoolGetDatum(true); replacesAtt[Anum_pg_attribute_atthasdef - 1] = true; - /* - * Note: this code is dead so far as core Postgres is concerned, - * because no caller passes add_column_mode = true anymore. We keep - * it in back branches on the slight chance that some extension is - * depending on it. - */ - if (rel->rd_rel->relkind == RELKIND_RELATION && add_column_mode && - !attgenerated) - { - expr2 = expression_planner(expr2); - estate = CreateExecutorState(); - exprState = ExecPrepareExpr(expr2, estate); - econtext = GetPerTupleExprContext(estate); - - missingval = ExecEvalExpr(exprState, econtext, - &missingIsNull); - - FreeExecutorState(estate); - - defAttStruct = TupleDescAttr(rel->rd_att, attnum - 1); - - if (missingIsNull) - { - /* if the default evaluates to NULL, just store a NULL array */ - missingval = (Datum) 0; - } - else - { - /* otherwise make a one-element array of the value */ - missingval = PointerGetDatum(construct_array(&missingval, - 1, - defAttStruct->atttypid, - defAttStruct->attlen, - defAttStruct->attbyval, - defAttStruct->attalign)); - } - - valuesAtt[Anum_pg_attribute_atthasmissing - 1] = !missingIsNull; - replacesAtt[Anum_pg_attribute_atthasmissing - 1] = true; - valuesAtt[Anum_pg_attribute_attmissingval - 1] = missingval; - replacesAtt[Anum_pg_attribute_attmissingval - 1] = true; - nullsAtt[Anum_pg_attribute_attmissingval - 1] = missingIsNull; - } atttup = heap_modify_tuple(atttup, RelationGetDescr(attrrel), valuesAtt, nullsAtt, replacesAtt); CatalogTupleUpdate(attrrel, &atttup->t_self, atttup); - - if (!missingIsNull) - pfree(DatumGetPointer(missingval)); } table_close(attrrel, RowExclusiveLock); heap_freetuple(atttup); diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index fd31216b27b..7b7b09baaa8 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -967,7 +967,6 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, rawEnt = (RawColumnDefault *) palloc(sizeof(RawColumnDefault)); rawEnt->attnum = attnum; rawEnt->raw_default = colDef->raw_default; - rawEnt->missingMode = false; rawEnt->generated = colDef->generated; rawDefaults = lappend(rawDefaults, rawEnt); attr->atthasdef = true; @@ -7321,7 +7320,6 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel, rawEnt = (RawColumnDefault *) palloc(sizeof(RawColumnDefault)); rawEnt->attnum = attribute->attnum; rawEnt->raw_default = copyObject(colDef->raw_default); - rawEnt->missingMode = false; /* XXX vestigial */ rawEnt->generated = colDef->generated; /* @@ -8077,7 +8075,6 @@ ATExecColumnDefault(Relation rel, const char *colName, rawEnt = (RawColumnDefault *) palloc(sizeof(RawColumnDefault)); rawEnt->attnum = attnum; rawEnt->raw_default = newDefault; - rawEnt->missingMode = false; rawEnt->generated = '\0'; /* @@ -8115,7 +8112,7 @@ ATExecCookedColumnDefault(Relation rel, AttrNumber attnum, RemoveAttrDefault(RelationGetRelid(rel), attnum, DROP_RESTRICT, false, true); - (void) StoreAttrDefault(rel, attnum, newDefault, true, false); + (void) StoreAttrDefault(rel, attnum, newDefault, true); ObjectAddressSubSet(address, RelationRelationId, RelationGetRelid(rel), attnum); @@ -8578,7 +8575,6 @@ ATExecSetExpression(AlteredTableInfo *tab, Relation rel, const char *colName, rawEnt = (RawColumnDefault *) palloc(sizeof(RawColumnDefault)); rawEnt->attnum = attnum; rawEnt->raw_default = newExpr; - rawEnt->missingMode = false; rawEnt->generated = attgenerated; /* Store the generated expression */ @@ -14130,7 +14126,7 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, RemoveAttrDefault(RelationGetRelid(rel), attnum, DROP_RESTRICT, true, true); - StoreAttrDefault(rel, attnum, defaultexpr, true, false); + (void) StoreAttrDefault(rel, attnum, defaultexpr, true); } ObjectAddressSubSet(address, RelationRelationId, diff --git a/src/include/catalog/heap.h b/src/include/catalog/heap.h index 1c67e329264..dbd339e9df4 100644 --- a/src/include/catalog/heap.h +++ b/src/include/catalog/heap.h @@ -29,7 +29,6 @@ typedef struct RawColumnDefault { AttrNumber attnum; /* attribute to attach default to */ Node *raw_default; /* default value (untransformed parse tree) */ - bool missingMode; /* obsolete, no longer used */ char generated; /* attgenerated setting */ } RawColumnDefault; diff --git a/src/include/catalog/pg_attrdef.h b/src/include/catalog/pg_attrdef.h index 192799cfed7..3067a8357c0 100644 --- a/src/include/catalog/pg_attrdef.h +++ b/src/include/catalog/pg_attrdef.h @@ -57,8 +57,7 @@ DECLARE_FOREIGN_KEY((adrelid, adnum), pg_attribute, (attrelid, attnum)); extern Oid StoreAttrDefault(Relation rel, AttrNumber attnum, - Node *expr, bool is_internal, - bool add_column_mode); + Node *expr, bool is_internal); extern void RemoveAttrDefault(Oid relid, AttrNumber attnum, DropBehavior behavior, bool complain, bool internal);