On Tue, 21 Mar 2023 at 12:26, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> On 2023-Mar-21, Dean Rasheed wrote:
>
> > Looking at it with fresh eyes though, I realise that I could have just written
> >
> > action->qual = make_and_qual((Node *) ntest, action->qual);
> >
> > which is equivalent, but more concise.
>
> Nice.
>
> I have no further observations about this patch.
>
Looking at this one afresh, it seems that the change to make Vars
outer-join aware broke it -- the Var in the qual to test whether the
source row is null needs to be marked as nullable by the join added by
transform_MERGE_to_join(). That's something that needs to be done in
transform_MERGE_to_join(), so it makes more sense to add the new qual
there rather than in transformMergeStmt().
Also, now that MERGE has ruleutils support, it's clear that adding the
qual in transformMergeStmt() isn't right anyway, since it would then
appear in the deparsed output.
So attached is an updated patch doing that, which seems neater all
round, since adding the qual is closely related to the join-type
choice, which is now a decision taken entirely in
transform_MERGE_to_join(). This requires a new "mergeSourceRelation"
field on the Query structure, but as before, it does away with the
"mergeUseOuterJoin" field.
I've also updated the ruleutils support. In the absence of any WHEN
NOT MATCHED BY SOURCE actions, this will output not-matched actions
simply as "WHEN NOT MATCHED" for backwards compatibility, and to be
SQL-standard-compliant. If there are any WHEN NOT MATCHED BY SOURCE
actions though, I think it's preferable to output explicit "BY SOURCE"
and "BY TARGET" qualifiers for all not-matched actions, to make the
meaning clearer.
Regards,
Dean