(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;
+ }

Reply via email to