Horton

Thread

Commits

  1. Reject MERGE in CTEs and COPY

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

    The Post Office <noreply@postgresql.org> — 2022-08-07T21:44:25Z

    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.

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-08-08T02:21:07Z

    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

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

    Richard Guo <guofenglinux@gmail.com> — 2022-08-11T02:41:37Z

    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

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

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2022-08-11T11:40:20Z

    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/

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-08-11T13:37:08Z

    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

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

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2022-08-11T14:17:45Z

    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)

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-08-11T14:23:15Z

    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

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

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2022-08-11T16:27:21Z

    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/

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-08-11T17:08:00Z

    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

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

    Richard Guo <guofenglinux@gmail.com> — 2022-08-12T00:58:28Z

    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