Thread: BUG #17579: 15beta2: strange error when trying to use MERGE statement as a CTE

The following bug has been logged on the website:

Bug reference:      17579
Logged by:          Alexey Borzov
Email address:      borz_off@cs.msu.su
PostgreSQL version: Unsupported/Unknown
Operating system:   Windows
Description:

create table foo (id int);

with cte_failure as (
    merge into foo as target
    using foo as source
    on target.id = source.id
    when matched then do nothing
)
select 'fail!';

When executing the above code I get the following error:

> ERROR:  DO INSTEAD NOTIFY rules are not supported for data-modifying
statements in WITH

I suspect that MERGE was never intended to work as a CTE, but right now the
grammar allows any PreparableStmt in a common_table_expr and the above error
is triggered a lot later due to implementation details.

PG Bug reporting form <noreply@postgresql.org> writes:
> create table foo (id int);

> with cte_failure as (
>     merge into foo as target
>     using foo as source
>     on target.id = source.id
>     when matched then do nothing
> )
> select 'fail!';

> When executing the above code I get the following error:

> ERROR:  DO INSTEAD NOTIFY rules are not supported for data-modifying
> statements in WITH

With asserts on, it fails in the parser:

TRAP: FailedAssertion("IsA(cte->ctequery, InsertStmt) || IsA(cte->ctequery, UpdateStmt) || IsA(cte->ctequery,
DeleteStmt)",File: "parse_cte.c", Line: 149, PID: 303950) 
postgres: postgres regression [local] SELECT(ExceptionalCondition+0x7c)[0x98013c]
postgres: postgres regression [local] SELECT(transformWithClause+0x66c)[0x6275ec]
postgres: postgres regression [local] SELECT(transformStmt+0x10f9)[0x603619]

> I suspect that MERGE was never intended to work as a CTE, but right now the
> grammar allows any PreparableStmt in a common_table_expr and the above error
> is triggered a lot later due to implementation details.

It evidently wasn't ever *tested*, but in principle I think it ought
to work.  I'm not sure how much effort will be involved to make that
happen.  At this point we might have to disallow it for v15 and
come back to the problem later.

            regards, tom lane

On 2022-Aug-11, Richard Guo wrote:

> Seems we neglect to think of MERGE statements when we transform WITH
> clauses and when we rewrite the query. If we add the check against MERGE
> statement in parse_cte.c and in rewriteHandler.c, the query in problem
> can work. But I'm not sure if that's enough.

I would like to have MERGE within CTEs, but I think for it to be truly
useful we need a RETURNING clause, which is currently not implemented.
I don't think it's terribly difficult to implement ... AFAICS most of
the pieces are there ... but clearly out of scope for pg15.

> +1 to disallow it for now.

This patch does that.

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

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> On 2022-Aug-11, Richard Guo wrote:
>> +1 to disallow it for now.

> This patch does that.

The parse error location seems quite oddly chosen.  Can you make
it point at the MERGE instead?  I think exprLocation(cte->ctequery)
might work, but not sure.

            regards, tom lane

In the meantime I noticed that DoCopy is subject to the same problem,
since it also uses PreparableStmt.  I'll post a patch in a bit.

On 2022-Aug-11, Tom Lane wrote:

> The parse error location seems quite oddly chosen.  Can you make
> it point at the MERGE instead?  I think exprLocation(cte->ctequery)
> might work, but not sure.

I tried, but couldn't find the location anywhere.  None of the nodes for
ModifyTable statements have location :-(  We could add one now -- while
it's a bit late for 15, we're likely going to have a catversion bump due
to the JSON revert, so it might be fine.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"Cuando mañana llegue pelearemos segun lo que mañana exija" (Mowgli)

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> On 2022-Aug-11, Tom Lane wrote:
>> The parse error location seems quite oddly chosen.  Can you make
>> it point at the MERGE instead?  I think exprLocation(cte->ctequery)
>> might work, but not sure.

> I tried, but couldn't find the location anywhere.  None of the nodes for
> ModifyTable statements have location :-(

Ah, right, in particular MergeStmt doesn't.

> We could add one now -- while
> it's a bit late for 15, we're likely going to have a catversion bump due
> to the JSON revert, so it might be fine.

Nah, not worth it just for this.

            regards, tom lane

Here's a patch.  I kept the location in the CTE bit, because it's better
than nothing.  No location hint is possible for the COPY one, as far as
I can see.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> Here's a patch.  I kept the location in the CTE bit, because it's better
> than nothing.  No location hint is possible for the COPY one, as far as
> I can see.

OK by me.

            regards, tom lane