On Fri, Nov 12, 2021 at 3:13 PM Alvaro Herrera <alvhe...@alvh.no-ip.org> wrote:
> On 2021-Nov-12, Zhihong Yu wrote: > > > Hi, > > > > + skipped_path = total - insert_path - update_path - > delete_path; > > > > Should there be an assertion that skipped_path is not negative ? > > Hm, yeah, added. > > > + * We maintain separate transaction tables for UPDATE/INSERT/DELETE > since > > + * MERGE can run all three actions in a single statement. Note that > UPDATE > > + * needs both old and new transition tables > > > > Should the 'transaction' in the first line be transition ? > > Oh, of course. > > Uploaded fixup commits to > https://github.com/alvherre/postgres/commits/merge-15 > > -- > Álvaro Herrera Valdivia, Chile — > https://www.EnterpriseDB.com/ Hi, + resultRelInfo->ri_notMatchedMergeAction = NIL; ri_notMatchedMergeAction -> ri_unmatchedMergeAction +static void ExecMergeNotMatched(ModifyTableState *mtstate, ExecMergeNotMatched -> ExecMergeUnmatched + * In this case, we are still dealing with a WHEN MATCHED case. + * In this case, we recheck the list of WHEN MATCHED actions from It seems the comment can be simplified to: + * In this case, since we are still dealing with a WHEN MATCHED case, + * we recheck the list of WHEN MATCHED actions from + * If we got no tuple, or the tuple we get has 'get' appears in different tenses. Better use either 'get' or 'got' in both places. +lmerge_matched: ... + foreach(l, resultRelInfo->ri_matchedMergeAction) I suggest expanding the foreach macro into the form of for loop where the loop condition has extra boolean variable merge_matched. Initial value for merge_matched can be true. Inside the loop, we can adjust merge_matched's value to control whether the for loop continues. This would avoid using goto label. + if (commandType == CMD_UPDATE && tuple_updated) Since commandType can only be update or delete, it seems tuple_updated and tuple_deleted can be consolidated into one boolean variable (tuple_modified). The above point is personal preference. + * We've activated one of the WHEN clauses, so we don't search + * further. This is required behaviour, not an optimization. + */ + break; We can directly return instead of break'ing. + * Similar logic appears in ExecInitPartitionInfo(), so if changing + * anything here, do so there too. The above implies code dedup via refactoring - can be done in a separate patch. To be continued ...
