From 84d5360fec2b598369d68d2c0101812488141adc Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 2 Mar 2025 16:56:00 -0500 Subject: [PATCH v3 2/2] Remove now-dead code in StoreAttrDefault(). 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 | 79 +++----------------------------- src/backend/commands/tablecmds.c | 8 +--- src/include/catalog/heap.h | 1 - src/include/catalog/pg_attrdef.h | 3 +- 5 files changed, 12 insertions(+), 84 deletions(-) diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index d28ed3eb396..1f9250aa065 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -2327,7 +2327,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 = @@ -2454,8 +2454,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..1f95bc6f752 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); @@ -92,7 +81,7 @@ StoreAttrDefault(Relation rel, AttrNumber attnum, /* * Update the pg_attribute entry for the column to show that a default - * exists. + * exists. (In some cases atthasdef will already be set.) */ attrrel = table_open(AttributeRelationId, RowExclusiveLock); atttup = SearchSysCacheCopy2(ATTNUM, @@ -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 cd90f2c7367..cf519fb77d9 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; /* @@ -8075,7 +8073,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'; /* @@ -8113,7 +8110,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); @@ -8576,7 +8573,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 */ @@ -14128,7 +14124,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 aa6aaaaa9d8..d8fbbf68a5b 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); -- 2.43.5