On 2018/04/06 5:00, Andres Freund wrote:
> On 2018-04-05 11:31:48 +0530, Pavan Deolasee wrote:
>>> + /*
>>> + * If there are not WHEN MATCHED actions, we are done.
>>> + */
>>> + if (mergeMatchedActionStates == NIL)
>>> + return true;
>>>
>>> Maybe I'm confused, but why is mergeMatchedActionStates attached to
>>> per-partition info? How can this differ in number between partitions,
>>> requiring us to re-check it below fetching the partition info above?
>>>
>>>
>> Because each partition may have a columns in different order, dropped
>> attributes etc. So we need to give treatment to the quals/targetlists. See
>> ON CONFLICT DO UPDATE for similar code.
>
> But the count wouldn't change, no? So we return before building the
> partition info if there's no MATCHED action?
Yeah, I think we should return at the top if there are no matched actions.
With the current code, even if there aren't any matched actions we're
doing ExecFindPartitionByOid() and ExecInitPartitionInfo() to only then
see in the partition's resultRelInfo that the action states list is empty.
I think there should be an Assert where there currently is the check for
empty action states list and the check itself should be at the top of the
function, as done in the attached.
Thanks,
Amit
diff --git a/src/backend/executor/execMerge.c b/src/backend/executor/execMerge.c
index d39ddd3034..2ed041644f 100644
--- a/src/backend/executor/execMerge.c
+++ b/src/backend/executor/execMerge.c
@@ -174,6 +174,12 @@ ExecMergeMatched(ModifyTableState *mtstate, EState *estate,
ListCell *l;
TupleTableSlot *saved_slot = slot;
+ /*
+ * If there are not WHEN MATCHED actions, we are done.
+ */
+ if (resultRelInfo->ri_mergeState->matchedActionStates == NIL)
+ return true;
+
if (mtstate->mt_partition_tuple_routing)
{
Datum datum;
@@ -220,12 +226,7 @@ ExecMergeMatched(ModifyTableState *mtstate, EState *estate,
*/
mergeMatchedActionStates =
resultRelInfo->ri_mergeState->matchedActionStates;
-
- /*
- * If there are not WHEN MATCHED actions, we are done.
- */
- if (mergeMatchedActionStates == NIL)
- return true;
+ Assert(mergeMatchedActionStates != NIL);
/*
* Make tuple and any needed join variables available to ExecQual and
