Re: [PATCH] Implement INSERT SET syntax

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: David Steele <david(at)pgmasters(dot)net>
Cc: Gareth Palmer <gareth(at)internetnz(dot)net(dot)nz>, Michael Paquier <michael(at)paquier(dot)xyz>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Vik Fearing <vik(dot)fearing(at)2ndquadrant(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] Implement INSERT SET syntax
Date: 2020-03-24 17:57:43
Message-ID: 21979.1585072663@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

David Steele <david(at)pgmasters(dot)net> writes:
> On 12/3/19 4:44 AM, Gareth Palmer wrote:
>> Attached is a fixed version.

> Does this version of the patch address your concerns?

No. I still find the reliance on a FROM clause being present
to be pretty arbitrary. Also, I don't believe that ruleutils.c
requires no changes, because it's not going to be possible to
transform every usage of this syntax to old-style. I tried to
prove the point with this trivial example:

regression=# create table foo (f1 int ,f2 int, f3 int);
CREATE TABLE
regression=# create table bar (f1 int ,f2 int, f3 int);
CREATE TABLE
regression=# create rule r1 as on insert to foo do instead
regression-# insert into bar set (f1,f2,f3) = (select f1,f2,f3 from foo);

intending to show that the rule decompilation was bogus, but
I didn't get that far because the parser crashed:

TRAP: FailedAssertion("pstate->p_multiassign_exprs == NIL", File: "parse_target.c", Line: 287)
postgres: postgres regression [local] CREATE RULE(ExceptionalCondition+0x55)[0x8fb6e5]
postgres: postgres regression [local] CREATE RULE[0x5bd0c3]
postgres: postgres regression [local] CREATE RULE[0x583def]
postgres: postgres regression [local] CREATE RULE(transformStmt+0x2d5)[0x582665]
postgres: postgres regression [local] CREATE RULE(transformRuleStmt+0x2ad)[0x5bf2ad]
postgres: postgres regression [local] CREATE RULE(DefineRule+0x17)[0x793847]

If I do it like this, I get a different assertion:

regression=# insert into bar set (f1,f2,f3) = (select f1,f2,f3) from foo;
server closed the connection unexpectedly

TRAP: FailedAssertion("exprKind == EXPR_KIND_UPDATE_SOURCE", File: "parse_target.c", Line: 209)
postgres: postgres regression [local] INSERT(ExceptionalCondition+0x55)[0x8fb6e5]
postgres: postgres regression [local] INSERT(transformTargetList+0x1a7)[0x5bd277]
postgres: postgres regression [local] INSERT(transformStmt+0xbe0)[0x582f70]
postgres: postgres regression [local] INSERT[0x5839f3]
postgres: postgres regression [local] INSERT(transformStmt+0x2d5)[0x582665]
postgres: postgres regression [local] INSERT(transformTopLevelStmt+0xd)[0x58411d]
postgres: postgres regression [local] INSERT(parse_analyze+0x69)[0x584269]

No doubt that's all fixable, but the realization that some cases of
this syntax are *not* just syntactic sugar for standards-compliant
syntax is giving me pause. Do we really want to get out front of
the SQL committee on extending INSERT in an incompatible way?

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Etsuro Fujita 2020-03-24 18:03:11 Re: [HACKERS] advanced partition matching algorithm for partition-wise join
Previous Message David Steele 2020-03-24 17:12:20 Re: Columns correlation and adaptive query optimization