On Sat, Nov 15, 2025 at 12:59 AM Bertrand Drouvot <[email protected]> wrote: > > Hi, > > On Fri, Nov 14, 2025 at 11:47:35PM +0800, jian he wrote: > > hi. > > > > if PartitionKeyData->partattrs is 0, then it means that partition key is > > expression, else it's column reference. > > > > we can change from > > if (key->partattrs[i] == 0) > > to > > yes, I think that makes sense to not compare an AttrNumber type to 0, but... > > > if (key->partattrs[i] == InvalidAttrNumber) > > I think that we should make use of AttributeNumberIsValid() instead. Same > spirit as we've done in a2b02293bc6. > hi.
AttributeNumberIsValid makes more sense. Attached changes on pg_partitioned_table, pg_index are based on AttributeNumberIsValid. > FWIW, I'm looking at the Oid/OidIsValid() cases currently (writing a > coccinelle > script). > > I can look at the AttrNumber/AttributeNumberIsValid() cases after (that > should be > as easy as changing Oid to AttrNumber and OidIsValid() to > AttributeNumberIsValid() > in the coccinelle script), unless you want to take care of the AttrNumber > case? > Currently, I am only interested in pg_partitioned_table.partattrs, pg_index.indkey. -- jian https://www.enterprisedb.com/
From 641f017028e7f55379955c26954af3f05e06bcf3 Mon Sep 17 00:00:00 2001 From: jian he <[email protected]> Date: Sun, 16 Nov 2025 23:47:30 +0800 Subject: [PATCH v2 2/2] refactor 0 to InvalidAttrNumber for pg_index.indkey associated discussion: https://postgr.es/m/cacjufxgmsphwx150cxmu6ksed-x2fmth3upcwhpedgmjv1b...@mail.gmail.com --- src/backend/access/index/genam.c | 2 +- src/backend/access/spgist/spgutils.c | 4 ++-- src/backend/commands/matview.c | 2 +- src/backend/commands/tablecmds.c | 2 +- src/backend/optimizer/path/indxpath.c | 6 +++--- src/backend/optimizer/plan/createplan.c | 4 ++-- src/backend/optimizer/util/plancat.c | 8 ++++---- src/backend/statistics/attribute_stats.c | 4 ++-- src/backend/utils/adt/ruleutils.c | 2 +- src/backend/utils/adt/selfuncs.c | 6 +++--- src/backend/utils/cache/relcache.c | 6 +++--- src/bin/pg_dump/pg_dump.c | 8 ++++---- 12 files changed, 27 insertions(+), 27 deletions(-) diff --git a/src/backend/access/index/genam.c b/src/backend/access/index/genam.c index 0cb27af1310..f0c06848fd6 100644 --- a/src/backend/access/index/genam.c +++ b/src/backend/access/index/genam.c @@ -225,7 +225,7 @@ BuildIndexValueDescription(Relation indexRelation, * to figure out what column(s) the expression includes and if the * user has SELECT rights on them. */ - if (attnum == InvalidAttrNumber || + if (!AttributeNumberIsValid(attnum) || pg_attribute_aclcheck(indrelid, attnum, GetUserId(), ACL_SELECT) != ACLCHECK_OK) { diff --git a/src/backend/access/spgist/spgutils.c b/src/backend/access/spgist/spgutils.c index 87c31da71a5..982975f529b 100644 --- a/src/backend/access/spgist/spgutils.c +++ b/src/backend/access/spgist/spgutils.c @@ -131,7 +131,7 @@ GetIndexInputType(Relation index, AttrNumber indexcol) if (!IsPolymorphicType(opcintype)) return opcintype; heapcol = index->rd_index->indkey.values[indexcol - 1]; - if (heapcol != 0) /* Simple index column? */ + if (AttributeNumberIsValid(heapcol)) /* Simple index column? */ return getBaseType(get_atttype(index->rd_index->indrelid, heapcol)); /* @@ -147,7 +147,7 @@ GetIndexInputType(Relation index, AttrNumber indexcol) indexpr_item = list_head(indexprs); for (int i = 1; i <= index->rd_index->indnkeyatts; i++) { - if (index->rd_index->indkey.values[i - 1] == 0) + if (!AttributeNumberIsValid(index->rd_index->indkey.values[i - 1])) { /* expression column */ if (indexpr_item == NULL) diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c index ef7c0d624f1..d8e390cee41 100644 --- a/src/backend/commands/matview.c +++ b/src/backend/commands/matview.c @@ -928,7 +928,7 @@ is_usable_unique_index(Relation indexRel) { int attnum = indexStruct->indkey.values[i]; - if (attnum <= 0) + if (!AttributeNumberIsValid(attnum) || attnum < 0) return false; } return true; diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index c0d743a1085..806ce249083 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -9001,7 +9001,7 @@ ATExecSetStatistics(Relation rel, const char *colName, int16 colNum, Node *newVa (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot alter statistics on included column \"%s\" of index \"%s\"", NameStr(attrtuple->attname), RelationGetRelationName(rel)))); - else if (rel->rd_index->indkey.values[attnum - 1] != 0) + else if (AttributeNumberIsValid(rel->rd_index->indkey.values[attnum - 1])) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot alter statistics on non-expression column \"%s\" of index \"%s\"", diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index edc6d2ac1d3..fe1b5fdbfaf 100644 --- a/src/backend/optimizer/path/indxpath.c +++ b/src/backend/optimizer/path/indxpath.c @@ -2279,7 +2279,7 @@ check_index_only(RelOptInfo *rel, IndexOptInfo *index) * For the moment, we just ignore index expressions. It might be nice * to do something with them, later. */ - if (attno == 0) + if (!AttributeNumberIsValid(attno)) continue; if (index->canreturn[i]) @@ -4383,7 +4383,7 @@ match_index_to_operand(Node *operand, operand = (Node *) ((RelabelType *) operand)->arg; indkey = index->indexkeys[indexcol]; - if (indkey != 0) + if (AttributeNumberIsValid(indkey)) { /* * Simple index column; operand must be a matching Var. @@ -4408,7 +4408,7 @@ match_index_to_operand(Node *operand, indexpr_item = list_head(index->indexprs); for (i = 0; i < indexcol; i++) { - if (index->indexkeys[i] == 0) + if (!AttributeNumberIsValid(index->indexkeys[i])) { if (indexpr_item == NULL) elog(ERROR, "wrong number of index expressions"); diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index 8af091ba647..1eeeee030d0 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -5117,7 +5117,7 @@ fix_indexqual_operand(Node *node, IndexOptInfo *index, int indexcol) Assert(indexcol >= 0 && indexcol < index->ncolumns); - if (index->indexkeys[indexcol] != 0) + if (AttributeNumberIsValid(index->indexkeys[indexcol])) { /* It's a simple index column */ if (IsA(node, Var) && @@ -5137,7 +5137,7 @@ fix_indexqual_operand(Node *node, IndexOptInfo *index, int indexcol) indexpr_item = list_head(index->indexprs); for (pos = 0; pos < index->ncolumns; pos++) { - if (index->indexkeys[pos] == 0) + if (!AttributeNumberIsValid(index->indexkeys[pos])) { if (indexpr_item == NULL) elog(ERROR, "too few entries in indexprs list"); diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c index 93a248c0459..f52c341942c 100644 --- a/src/backend/optimizer/util/plancat.c +++ b/src/backend/optimizer/util/plancat.c @@ -961,7 +961,7 @@ infer_arbiter_indexes(PlannerInfo *root) { int attno = idxRel->rd_index->indkey.values[natt]; - if (attno != 0) + if (AttributeNumberIsValid(attno)) indexedAttrs = bms_add_member(indexedAttrs, attno - FirstLowInvalidHeapAttributeNumber); } @@ -1107,7 +1107,7 @@ infer_collation_opclass_match(InferenceElem *elem, Relation idxRel, Oid collation = idxRel->rd_indcollation[natt - 1]; int attno = idxRel->rd_index->indkey.values[natt - 1]; - if (attno != 0) + if (AttributeNumberIsValid(attno)) nplain++; if (elem->inferopclass != InvalidOid && @@ -1130,7 +1130,7 @@ infer_collation_opclass_match(InferenceElem *elem, Relation idxRel, if (((Var *) elem->expr)->varattno == attno) return true; } - else if (attno == 0) + else if (!AttributeNumberIsValid(attno)) { Node *nattExpr = list_nth(idxExprs, (natt - 1) - nplain); @@ -2027,7 +2027,7 @@ build_index_tlist(PlannerInfo *root, IndexOptInfo *index, int indexkey = index->indexkeys[i]; Expr *indexvar; - if (indexkey != 0) + if (AttributeNumberIsValid(indexkey)) { /* simple column */ const FormData_pg_attribute *att_tup; diff --git a/src/backend/statistics/attribute_stats.c b/src/backend/statistics/attribute_stats.c index ef4d768feab..ac94ff8b5f6 100644 --- a/src/backend/statistics/attribute_stats.c +++ b/src/backend/statistics/attribute_stats.c @@ -566,13 +566,13 @@ get_attr_expr(Relation rel, int attnum) * The index attnum points directly to a relation attnum, then it's not an * expression attribute. */ - if (rel->rd_index->indkey.values[attnum - 1] != 0) + if (AttributeNumberIsValid(rel->rd_index->indkey.values[attnum - 1])) return NULL; indexpr_item = list_head(rel->rd_indexprs); for (int i = 0; i < attnum - 1; i++) - if (rel->rd_index->indkey.values[i] == 0) + if (!AttributeNumberIsValid(rel->rd_index->indkey.values[i] == 0)) indexpr_item = lnext(rel->rd_indexprs, indexpr_item); if (indexpr_item == NULL) /* shouldn't happen */ diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index c4e0193d317..51a76cb2d74 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -1418,7 +1418,7 @@ pg_get_indexdef_worker(Oid indexrelid, int colno, appendStringInfoString(&buf, sep); sep = ", "; - if (attnum != 0) + if (AttributeNumberIsValid(attnum)) { /* Simple index column */ char *attname; diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c index cb23ad52782..1a148de406e 100644 --- a/src/backend/utils/adt/selfuncs.c +++ b/src/backend/utils/adt/selfuncs.c @@ -5440,7 +5440,7 @@ examine_variable(PlannerInfo *root, Node *node, int varRelid, for (pos = 0; pos < index->ncolumns; pos++) { - if (index->indexkeys[pos] == 0) + if (!AttributeNumberIsValid(index->indexkeys[pos])) { Node *indexkey; @@ -6109,7 +6109,7 @@ examine_indexcol_variable(PlannerInfo *root, IndexOptInfo *index, AttrNumber colnum; Oid relid; - if (index->indexkeys[indexcol] != 0) + if (AttributeNumberIsValid(index->indexkeys[indexcol])) { /* Simple variable --- look to stats for the underlying table */ RangeTblEntry *rte = planner_rt_fetch(index->rel->relid, root); @@ -8671,7 +8671,7 @@ brincostestimate(PlannerInfo *root, IndexPath *path, double loop_count, AttrNumber attnum = index->indexkeys[iclause->indexcol]; /* attempt to lookup stats in relation for this index column */ - if (attnum != 0) + if (AttributeNumberIsValid(attnum)) { /* Simple variable -- look to stats for the underlying table */ if (get_relation_stats_hook && diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 915d0bc9084..2f84eda70d5 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -5436,7 +5436,7 @@ restart: /* Collect simple attribute references */ for (i = 0; i < indexDesc->rd_index->indnatts; i++) { - int attrnum = indexDesc->rd_index->indkey.values[i]; + AttrNumber attrnum = indexDesc->rd_index->indkey.values[i]; /* * Since we have covering indexes with non-key columns, we must @@ -5452,7 +5452,7 @@ restart: * key or identity key. Hence we do not include them into * uindexattrs, pkindexattrs and idindexattrs bitmaps. */ - if (attrnum != 0) + if (AttributeNumberIsValid(attrnum)) { *attrs = bms_add_member(*attrs, attrnum - FirstLowInvalidHeapAttributeNumber); @@ -5614,7 +5614,7 @@ RelationGetIdentityKeyBitmap(Relation relation) * We don't include non-key columns into idindexattrs bitmaps. See * RelationGetIndexAttrBitmap. */ - if (attrnum != 0) + if (AttributeNumberIsValid(attrnum)) { if (i < indexDesc->rd_index->indnkeyatts) idindexattrs = bms_add_member(idindexattrs, diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index a00918bacb4..f44a3ab6702 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -18582,10 +18582,10 @@ dumpConstraint(Archive *fout, const ConstraintInfo *coninfo) appendPQExpBufferStr(q, " ("); for (k = 0; k < indxinfo->indnkeyattrs; k++) { - int indkey = (int) indxinfo->indkeys[k]; + AttrNumber indkey = (AttrNumber) indxinfo->indkeys[k]; const char *attname; - if (indkey == InvalidAttrNumber) + if (!AttributeNumberIsValid(indkey)) break; attname = getAttrName(indkey, tbinfo); @@ -18601,10 +18601,10 @@ dumpConstraint(Archive *fout, const ConstraintInfo *coninfo) for (k = indxinfo->indnkeyattrs; k < indxinfo->indnattrs; k++) { - int indkey = (int) indxinfo->indkeys[k]; + AttrNumber indkey = (AttrNumber) indxinfo->indkeys[k]; const char *attname; - if (indkey == InvalidAttrNumber) + if (!AttributeNumberIsValid(indkey)) break; attname = getAttrName(indkey, tbinfo); -- 2.34.1
From ae0b5af8c1fb417e3a80cc05d32880fd7273f998 Mon Sep 17 00:00:00 2001 From: jian he <[email protected]> Date: Sun, 16 Nov 2025 23:54:15 +0800 Subject: [PATCH v2 1/2] refactor 0 to InvalidAttrNumber for pg_partitioned_table.partattrs discussion: https://postgr.es/m/cacjufxgmsphwx150cxmu6ksed-x2fmth3upcwhpedgmjv1b...@mail.gmail.com --- src/backend/catalog/heap.c | 2 +- src/backend/commands/indexcmds.c | 2 +- src/backend/commands/tablecmds.c | 2 +- src/backend/executor/execPartition.c | 2 +- src/backend/optimizer/util/plancat.c | 2 +- src/backend/parser/parse_utilcmd.c | 4 ++-- src/backend/partitioning/partbounds.c | 8 ++++---- src/backend/utils/adt/ruleutils.c | 2 +- src/backend/utils/cache/partcache.c | 2 +- src/include/utils/partcache.h | 2 +- 10 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index fd6537567ea..23198161cbb 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -3981,7 +3981,7 @@ StorePartitionKey(Relation rel, */ for (i = 0; i < partnatts; i++) { - if (partattrs[i] == 0) + if (!AttributeNumberIsValid(partattrs[i])) continue; /* ignore expressions here */ ObjectAddressSubSet(referenced, RelationRelationId, diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 5712fac3697..221c7d1d607 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -1012,7 +1012,7 @@ DefineIndex(Oid tableId, * It may be possible to support UNIQUE constraints when partition * keys are expressions, but is it worth it? Give up for now. */ - if (key->partattrs[i] == 0) + if (!AttributeNumberIsValid(key->partattrs[i])) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("unsupported %s constraint with partition key definition", diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 23ebaa3f230..c0d743a1085 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -19920,7 +19920,7 @@ ComputePartitionAttrs(ParseState *pstate, Relation rel, List *partParams, AttrNu } else { - partattrs[attn] = 0; /* marks the column as expression */ + partattrs[attn] = InvalidAttrNumber; /* marks the column as expression */ *partexprs = lappend(*partexprs, expr); /* diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c index aa12e9ad2ea..6272fb5f214 100644 --- a/src/backend/executor/execPartition.c +++ b/src/backend/executor/execPartition.c @@ -1325,7 +1325,7 @@ FormPartitionKeyDatum(PartitionDispatch pd, Datum datum; bool isNull; - if (keycol != 0) + if (AttributeNumberIsValid(keycol)) { /* Plain column; get the value directly from the heap tuple */ datum = slot_getattr(slot, keycol, &isNull); diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c index d950bd93002..93a248c0459 100644 --- a/src/backend/optimizer/util/plancat.c +++ b/src/backend/optimizer/util/plancat.c @@ -2699,7 +2699,7 @@ set_baserel_partition_key_exprs(Relation relation, Expr *partexpr; AttrNumber attno = partkey->partattrs[cnt]; - if (attno != InvalidAttrNumber) + if (AttributeNumberIsValid(attno)) { /* Single column partition key is stored as a Var node. */ Assert(attno > 0); diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c index e96b38a59d5..a67761e81d3 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -4345,7 +4345,7 @@ transformPartitionBound(ParseState *pstate, Relation parent, parser_errposition(pstate, exprLocation((Node *) spec)))); /* Get the only column's name in case we need to output an error */ - if (key->partattrs[0] != 0) + if (AttributeNumberIsValid(key->partattrs[0])) colname = get_attname(RelationGetRelid(parent), key->partattrs[0], false); else @@ -4494,7 +4494,7 @@ transformPartitionRangeBounds(ParseState *pstate, List *blist, Const *value; /* Get the column's name in case we need to output an error */ - if (key->partattrs[i] != 0) + if (AttributeNumberIsValid(key->partattrs[i])) colname = get_attname(RelationGetRelid(parent), key->partattrs[i], false); else diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.c index 8ba038c5ef4..e09cd1c1a95 100644 --- a/src/backend/partitioning/partbounds.c +++ b/src/backend/partitioning/partbounds.c @@ -4026,7 +4026,7 @@ get_qual_for_hash(Relation parent, PartitionBoundSpec *spec) Node *keyCol; /* Left operand */ - if (key->partattrs[i] != 0) + if (AttributeNumberIsValid(key->partattrs[i])) { keyCol = (Node *) makeVar(1, key->partattrs[i], @@ -4082,7 +4082,7 @@ get_qual_for_list(Relation parent, PartitionBoundSpec *spec) Assert(key->partnatts == 1); /* Construct Var or expression representing the partition column */ - if (key->partattrs[0] != 0) + if (AttributeNumberIsValid(key->partattrs[0])) keyCol = (Expr *) makeVar(1, key->partattrs[0], key->parttypid[0], @@ -4638,7 +4638,7 @@ get_range_key_properties(PartitionKey key, int keynum, Const **lower_val, Const **upper_val) { /* Get partition key expression for this column */ - if (key->partattrs[keynum] != 0) + if (AttributeNumberIsValid(key->partattrs[keynum])) { *keyCol = (Expr *) makeVar(1, key->partattrs[keynum], @@ -4686,7 +4686,7 @@ get_range_nulltest(PartitionKey key) { Expr *keyCol; - if (key->partattrs[i] != 0) + if (AttributeNumberIsValid(key->partattrs[i])) { keyCol = (Expr *) makeVar(1, key->partattrs[i], diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 556ab057e5a..c4e0193d317 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -2031,7 +2031,7 @@ pg_get_partkeydef_worker(Oid relid, int prettyFlags, appendStringInfoString(&buf, sep); sep = ", "; - if (attnum != 0) + if (AttributeNumberIsValid(attnum)) { /* Simple attribute reference */ char *attname; diff --git a/src/backend/utils/cache/partcache.c b/src/backend/utils/cache/partcache.c index f5d7d70def0..8bcf77d55a5 100644 --- a/src/backend/utils/cache/partcache.c +++ b/src/backend/utils/cache/partcache.c @@ -226,7 +226,7 @@ RelationBuildPartitionKey(Relation relation) key->partcollation[i] = collation->values[i]; /* Collect type information */ - if (attno != 0) + if (AttributeNumberIsValid(attno)) { Form_pg_attribute att = TupleDescAttr(relation->rd_att, attno - 1); diff --git a/src/include/utils/partcache.h b/src/include/utils/partcache.h index 0fb6fc1c808..8a9d07c03c8 100644 --- a/src/include/utils/partcache.h +++ b/src/include/utils/partcache.h @@ -27,7 +27,7 @@ typedef struct PartitionKeyData PartitionStrategy strategy; /* partitioning strategy */ int16 partnatts; /* number of columns in the partition key */ AttrNumber *partattrs; /* attribute numbers of columns in the - * partition key or 0 if it's an expr */ + * partition key or InvalidAttrNumber (0) if it's an expr */ List *partexprs; /* list of expressions in the partitioning * key, one for each zero-valued partattrs */ -- 2.34.1
