(2018/03/18 13:17), Alvaro Herrera wrote:
Alvaro Herrera wrote:
The only thing that I remain unhappy about this patch is the whole
adjust_and_expand_partition_tlist() thing. I fear we may be doing
redundant and/or misplaced work. I'll look into it next week.
I'm still reviewing the patches, but I really agree on that point. As
Pavan mentioned upthread, the onConflictSet tlist for the root parent,
from which we create a translated onConflictSet tlist for a partition,
would have already been processed by expand_targetlist() to contain all
missing columns as well, so I think we could create the tlist for the
partition by simply re-ordering the expression-converted tlist (ie,
conv_setproj) based on the conversion map for the partition. The
Attached defines a function for that, which could be called, instead of
calling adjust_and_expand_partition_tlist(). This would allow us to get
rid of planner changes from the patches. Maybe I'm missing something,
though.
Best regards,
Etsuro Fujita
*** src/backend/executor/execPartition.c.org 2018-03-19 21:32:52.000000000 +0900
--- src/backend/executor/execPartition.c 2018-03-19 21:44:17.000000000 +0900
***************
*** 15,24 ****
--- 15,26 ----
#include "postgres.h"
#include "catalog/pg_inherits_fn.h"
+ #include "catalog/pg_type.h"
#include "executor/execPartition.h"
#include "executor/executor.h"
#include "mb/pg_wchar.h"
#include "miscadmin.h"
+ #include "nodes/makefuncs.h"
#include "optimizer/prep.h"
#include "utils/lsyscache.h"
#include "utils/rls.h"
***************
*** 37,42 ****
--- 39,45 ----
Datum *values,
bool *isnull,
int maxfieldlen);
+ static List *adjust_onconflictset_tlist(List *tlist, TupleConversionMap *map);
/*
* ExecSetupPartitionTupleRouting - sets up information needed during
***************
*** 554,565 ****
firstVarno, partrel,
firstResultRel, NULL);
! conv_setproj =
! adjust_and_expand_partition_tlist(RelationGetDescr(firstResultRel),
! RelationGetDescr(partrel),
! RelationGetRelationName(partrel),
! firstVarno,
! conv_setproj);
tupDesc = ExecTypeFromTL(conv_setproj, partrelDesc->tdhasoid);
part_conflproj_slot = ExecInitExtraTupleSlot(mtstate->ps.state,
--- 557,563 ----
firstVarno, partrel,
firstResultRel, NULL);
! conv_setproj = adjust_onconflictset_tlist(conv_setproj, map);
tupDesc = ExecTypeFromTL(conv_setproj, partrelDesc->tdhasoid);
part_conflproj_slot = ExecInitExtraTupleSlot(mtstate->ps.state,
***************
*** 1091,1093 ****
--- 1089,1153 ----
return buf.data;
}
+
+ /*
+ * Adjust the targetlist entries of a translated ON CONFLICT UPDATE operation
+ *
+ * The expressions have already been fixed, but we need to re-order the tlist
+ * so that the target resnos match the child table.
+ *
+ * The given tlist has already been through expression_tree_mutator;
+ * therefore the TargetEntry nodes are fresh copies that it's okay to
+ * scribble on.
+ */
+ static List *
+ adjust_onconflictset_tlist(List *tlist, TupleConversionMap *map)
+ {
+ List *new_tlist = NIL;
+ TupleDesc tupdesc = map->outdesc;
+ AttrNumber *attrMap = map->attrMap;
+ int numattrs = tupdesc->natts;
+ int attrno;
+
+ for (attrno = 1; attrno <= numattrs; attrno++)
+ {
+ Form_pg_attribute att_tup = TupleDescAttr(tupdesc, attrno - 1);
+ TargetEntry *tle;
+
+ if (attrMap[attrno - 1] != 0)
+ {
+ Assert(!att_tup->attisdropped);
+
+ /* Get the corresponding tlist entry from the given tlist */
+ tle = (TargetEntry *) list_nth(tlist, attrMap[attrno - 1] - 1);
+
+ /* Get the resno right */
+ if (tle->resno != attrno)
+ tle->resno = attrno;
+ }
+ else
+ {
+ Node *expr;
+
+ Assert(att_tup->attisdropped);
+
+ /* Insert NULL for dropped column */
+ expr = (Node *) makeConst(INT4OID,
+ -1,
+ InvalidOid,
+ sizeof(int32),
+ (Datum) 0,
+ true, /* isnull */
+ true /* byval */ );
+
+ tle = makeTargetEntry((Expr *) expr,
+ attrno,
+ pstrdup(NameStr(att_tup->attname)),
+ false);
+ }
+
+ new_tlist = lappend(new_tlist, tle);
+ }
+
+ return new_tlist;
+ }