Re: support for MERGE

From: Japin Li <japinli(at)hotmail(dot)com>
To: Erik Rijkers <er(at)xs4all(dot)nl>
Cc: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Andrew Dunstan <andrew(at)dunslane(dot)net>, Simon Riggs <simon(dot)riggs(at)enterprisedb(dot)com>, Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>, Zhihong Yu <zyu(at)yugabyte(dot)com>, Daniel Westermann <dwe(at)dbi-services(dot)com>, Amit Langote <amitlangote09(at)gmail(dot)com>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, Pavan Deolasee <pavan(dot)deolasee(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: support for MERGE
Date: 2022-01-16 18:04:03
Message-ID: MEYP282MB16695D606207B2090372E374B6569@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


On Fri, 14 Jan 2022 at 17:11, Erik Rijkers <er(at)xs4all(dot)nl> wrote:
> I got into this crash (may be the same as Jaime's):
>
> #-----
> # bash
>
> t1=table1
> t2=table2
>
> psql -qX << SQL
> drop table if exists $t1 cascade;
> drop table if exists $t2 cascade;
> create table $t1 as select /*cast(id::text as jsonb) js,*/ id from
> generate_series(1,20) as f(id);
> create table $t2 as select /*cast(id::text as jsonb) js,*/ id from
> generate_series(1,20) as f(id);
> delete from $t1 where id % 2 = 1;
> delete from $t2 where id % 2 = 0;
> ( select 't1 - target', count(*) t1 from $t1
> union all select 't2 - source', count(*) t2 from $t2 ) order by 1;
>
> merge into $t1 as t1 using $t2 as t2 on t1.id = t2.id
> when not matched and (t2.id > 10) and (t1.id > 10)
> then do nothing
> when not matched then insert values ( id )
> when matched then do nothing ;
>
> ( select 't1 - target', count(*) t1 from $t1
> union all select 't2 - source', count(*) t2 from $t2 ) order by 1 ;
>
> SQL
> #-----
>
> Referencing alias 't1' in the WHEN NOT MATCHED member seems what
> triggers his crash: when I remove the phrase 'and (t1.id > 10)', the
> statement finishes correctly.
>

The MERGE documentation says:

A condition on a WHEN MATCHED clause can refer to columns in both the source
and the target relations. A condition on a WHEN NOT MATCHED clause can only
refer to columns from the source relation, since by definition there is no
matching target row. Only the system attributes from the target table are
accessible.

So for NOT MATCHED, we are expected not use the target table columns.

The code comes from execMerge.c says:

/*
* Make source tuple available to ExecQual and ExecProject. We don't need
* the target tuple, since the WHEN quals and the targetlist can't refer to
* the target columns.
*/
econtext->ecxt_scantuple = NULL;
econtext->ecxt_innertuple = slot;
econtext->ecxt_outertuple = NULL;

It will set econtext->ecxt_scantuple to NULL, which leads the crash.

>
> And I don't know if it is related but if I use this phrase:
>
> when not matched and (id > 10)
>
> I get:
>
> ERROR: column "id" does not exist
> LINE 2: when not matched and id > 0 -- (t2.id > 10) and (t1.id > 10)
> ^
> HINT: There is a column named "id" in table "t1", but it cannot be
> referenced from this part of the query.
>
> Is that hint correct? Seems a bit strange.

I find the setNamespaceForMergeWhen() do not set the visiblity for RTE if
the command type is CMD_NOTHING, and both target and source tables can
be accessible for CMD_NOTHING.
Should we setNamespaceVisibilityForRTE() for CMD_NOTHING? I try to set it
and it works as expected. OTOH, the system attributes from target table
also cannot be accessible. I'm not sure the v6 patch how to implement this
limitation.

Here is my modification:

diff --git a/src/backend/parser/parse_merge.c b/src/backend/parser/parse_merge.c
index a3514053d4..415113157d 100644
--- a/src/backend/parser/parse_merge.c
+++ b/src/backend/parser/parse_merge.c
@@ -172,6 +172,18 @@ setNamespaceForMergeWhen(ParseState *pstate, MergeWhenClause *mergeWhenClause)
break;

case CMD_NOTHING:
+ /*
+ * We can use the WHEN condition for DO NOTHING, so we should
+ * make sure the relation can be visible for certain action.
+ */
+ if (mergeWhenClause->matched)
+ setNamespaceVisibilityForRTE(pstate->p_namespace,
+ targetRelRTE, true, true);
+ else
+ setNamespaceVisibilityForRTE(pstate->p_namespace,
+ targetRelRTE, false, false);
+ setNamespaceVisibilityForRTE(pstate->p_namespace,
+ sourceRelRTE, true, true);
break;
default:
elog(ERROR, "unknown action in MERGE WHEN clause");

Thoughts?

Attached v7 patch. Fix some typos from [1].

[1] https://www.postgresql.org/message-id/7d7a5e1b-402a-5685-2c28-2f4e44ad186d%40xs4all.nl

--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.

Attachment Content-Type Size
v7-0001-MERGE-SQL-Command-following-SQL-2016.patch text/x-patch 400.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2022-01-16 18:30:08 Re: Isn't wait_for_catchup slightly broken?
Previous Message Magnus Hagander 2022-01-16 16:39:11 Re: pg_basebackup WAL streamer shutdown is bogus - leading to slow tests