public inbox for [email protected]
help / color / mirror / Atom feedFrom: jian he <[email protected]>
To: PostgreSQL-development <[email protected]>
Subject: Re: refactor ExecInitPartitionInfo
Date: Sat, 21 Feb 2026 23:07:52 +0800
Message-ID: <CACJufxGcUztOYcCj85o15YT68qPY3qpwUtdndqDgOs_Sps6Cow@mail.gmail.com> (raw)
In-Reply-To: <CACJufxEN_mmgTtp-raJ9-VJHgyAGmb0SrO+01kn5by4mJ_XOfw@mail.gmail.com>
References: <CACJufxEN_mmgTtp-raJ9-VJHgyAGmb0SrO+01kn5by4mJ_XOfw@mail.gmail.com>
On Fri, Nov 28, 2025 at 7:00 PM jian he <[email protected]> wrote:
>
> hi.
>
> I noticed the following code pattern repeated several times in
> ExecInitPartitionInfo.
> ```
> if (part_attmap == NULL)
> build_attrmap_by_name(RelationGetDescr(partrel),
> RelationGetDescr(firstResultRel),
> false);
> ```
>
> we can consolidated into one, like:
>
> + if (node != NULL &&
> + (list_length(node->withCheckOptionLists) > 0 ||
> + list_length(node->returningLists) > 0 ||
> + node->onConflictAction != ONCONFLICT_NONE ||
> + node->operation == CMD_MERGE))
> + {
> + part_attmap =
> + build_attrmap_by_name(RelationGetDescr(partrel),
> + RelationGetDescr(firstResultRel),
> + false);
> + }
> +
>
> + if (node != NULL &&
> + (list_length(node->withCheckOptionLists) > 0 ||
> + list_length(node->returningLists) > 0 ||
> + node->onConflictAction != ONCONFLICT_NONE ||
> + node->operation == CMD_MERGE))
V1 is way too complicated.
IMHO, we can just do
if (node != NULL)
part_attmap = build_attrmap_by_name(RelationGetDescr(partrel),
RelationGetDescr(firstResultRel),
false);
We have now consolidated five uses of build_attrmap_by_name into one.
--
jian
https://www.enterprisedb.com/
Attachments:
[text/x-patch] v2-0001-refactor-ExecInitPartitionInfo.patch (3.3K, 2-v2-0001-refactor-ExecInitPartitionInfo.patch)
download | inline diff:
From 3b7eb284241675a1026eeebc9a6f682a88f95430 Mon Sep 17 00:00:00 2001
From: jian he <[email protected]>
Date: Sat, 21 Feb 2026 22:50:55 +0800
Subject: [PATCH v2 1/1] refactor ExecInitPartitionInfo
Discussion: https://postgr.es/m/CACJufxEN_mmgTtp-raJ9-VJHgyAGmb0SrO+01kn5by4mJ_XOfw@mail.gmail.com
commitfest: https://commitfest.postgresql.org/patch/6280
---
src/backend/executor/execPartition.c | 35 +++++-----------------------
1 file changed, 6 insertions(+), 29 deletions(-)
diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c
index bab294f5e91..d62d6a71f03 100644
--- a/src/backend/executor/execPartition.c
+++ b/src/backend/executor/execPartition.c
@@ -607,6 +607,11 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate,
(node != NULL &&
node->onConflictAction != ONCONFLICT_NONE));
+ if (node != NULL)
+ part_attmap = build_attrmap_by_name(RelationGetDescr(partrel),
+ RelationGetDescr(firstResultRel),
+ false);
+
/*
* Build WITH CHECK OPTION constraints for the partition. Note that we
* didn't build the withCheckOptionList for partitions within the planner,
@@ -649,10 +654,6 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate,
/*
* Convert Vars in it to contain this partition's attribute numbers.
*/
- part_attmap =
- build_attrmap_by_name(RelationGetDescr(partrel),
- RelationGetDescr(firstResultRel),
- false);
wcoList = (List *)
map_variable_attnos((Node *) wcoList,
firstVarno, 0,
@@ -706,14 +707,6 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate,
*/
returningList = linitial(node->returningLists);
- /*
- * Convert Vars in it to contain this partition's attribute numbers.
- */
- if (part_attmap == NULL)
- part_attmap =
- build_attrmap_by_name(RelationGetDescr(partrel),
- RelationGetDescr(firstResultRel),
- false);
returningList = (List *)
map_variable_attnos((Node *) returningList,
firstVarno, 0,
@@ -951,11 +944,7 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate,
List *onconflcols;
onconflset = copyObject(node->onConflictSet);
- if (part_attmap == NULL)
- part_attmap =
- build_attrmap_by_name(RelationGetDescr(partrel),
- RelationGetDescr(firstResultRel),
- false);
+
onconflset = (List *)
map_variable_attnos((Node *) onconflset,
INNER_VAR, 0,
@@ -1007,12 +996,6 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate,
{
List *clause;
- if (part_attmap == NULL)
- part_attmap =
- build_attrmap_by_name(RelationGetDescr(partrel),
- RelationGetDescr(firstResultRel),
- false);
-
clause = copyObject((List *) node->onConflictWhere);
clause = (List *)
map_variable_attnos((Node *) clause,
@@ -1064,12 +1047,6 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate,
ExprContext *econtext = mtstate->ps.ps_ExprContext;
Node *joinCondition;
- if (part_attmap == NULL)
- part_attmap =
- build_attrmap_by_name(RelationGetDescr(partrel),
- RelationGetDescr(firstResultRel),
- false);
-
if (unlikely(!leaf_part_rri->ri_projectNewInfoValid))
ExecInitMergeTupleSlots(mtstate, leaf_part_rri);
--
2.34.1
view thread (4+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected]
Subject: Re: refactor ExecInitPartitionInfo
In-Reply-To: <CACJufxGcUztOYcCj85o15YT68qPY3qpwUtdndqDgOs_Sps6Cow@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox