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 Mon, Aug 8, 2022 at 10:21 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
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.
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.
+1 to disallow it for now.
Thanks
Richard
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/
Attachments:
0001-Reject-MERGE-in-CTE.patchtext/x-diff; charset=us-asciiDownload+20-1
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/
Attachments:
v2-0001-Reject-MERGE-in-CTEs-and-COPY.patchtext/x-diff; charset=us-asciiDownload+37-1
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
On Fri, Aug 12, 2022 at 1:08 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
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.
+1. The patch looks good to me.
Thanks
Richard