Re: MERGE ... WHEN NOT MATCHED BY SOURCE

From: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
To: Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: MERGE ... WHEN NOT MATCHED BY SOURCE
Date: 2023-01-04 11:57:58
Message-ID: 20230104115758.2ajc235gphwfe3d6@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I haven't read this patch other than superficially; I suppose the
feature it's introducing is an OK one to have as an extension to the
standard. (I hope the community members that are committee members
will propose this extension to become part of the standard.)

On 2023-Jan-02, Dean Rasheed wrote:

> --- a/src/backend/optimizer/prep/preptlist.c
> +++ b/src/backend/optimizer/prep/preptlist.c
> @@ -157,15 +157,14 @@ preprocess_targetlist(PlannerInfo *root)
> /*
> * Add resjunk entries for any Vars used in each action's
> * targetlist and WHEN condition that belong to relations other
> - * than target. Note that aggregates, window functions and
> - * placeholder vars are not possible anywhere in MERGE's WHEN
> - * clauses. (PHVs may be added later, but they don't concern us
> - * here.)
> + * than target. Note that aggregates and window functions are not
> + * possible anywhere in MERGE's WHEN clauses, but PlaceHolderVars
> + * may have been added by subquery pullup.
> */
> vars = pull_var_clause((Node *)
> list_concat_copy((List *) action->qual,
> action->targetList),
> - 0);
> + PVC_INCLUDE_PLACEHOLDERS);

Hmm, is this new because of NOT MATCHED BY SOURCE, or is it something
that can already be hit by existing features of MERGE? In other words
-- is this a bug fix that should be backpatched ahead of introducing NOT
MATCHED BY SOURCE?

> @@ -127,10 +143,12 @@ transformMergeStmt(ParseState *pstate, M
> */
> is_terminal[0] = false;
> is_terminal[1] = false;
> + is_terminal[2] = false;

I think these 0/1/2 should be replaced by the values of MergeMatchKind.

> + /* Join type required */
> + if (left_join && right_join)
> + qry->mergeJoinType = JOIN_FULL;
> + else if (left_join)
> + qry->mergeJoinType = JOIN_LEFT;
> + else if (right_join)
> + qry->mergeJoinType = JOIN_RIGHT;
> + else
> + qry->mergeJoinType = JOIN_INNER;

One of the review comments that MERGE got initially was that parse
analysis was not a place to "do query optimization", in the sense that
the original code was making a decision whether to make an outer or
inner join based on the set of WHEN clauses that appear in the command.
That's how we ended up with transform_MERGE_to_join and
mergeUseOuterJoin instead. This new code is certainly not the same, but
it makes me a bit unconfortable. Maybe it's OK, though.

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Geier 2023-01-04 12:02:05 Re: Reduce timing overhead of EXPLAIN ANALYZE using rdtsc?
Previous Message tushar 2023-01-04 11:57:55 Getting an error if we provide --enable-tap-tests switch on SLES 12