Re: [PATCH] Implement INSERT SET syntax
As discussed in [1], we're taking this opportunity to return some patchsets that don't appear to be getting enough reviewer interest. This is not a rejection, since we don't necessarily think there's anything unacceptable about the entry, but it differs from a standard "Returned with Feedback" in that there's probably not much actionable feedback at all. Rather than code changes, what this patch needs is more community interest. You might - ask people for help with your approach, - see if there are similar patches that your code could supplement, - get interested parties to agree to review your patch in a CF, or - possibly present the functionality in a way that's easier to review overall. (Doing these things is no guarantee that there will be interest, but it's hopefully better than endlessly rebasing a patchset that is not receiving any feedback from the community.) Once you think you've built up some community support and the patchset is ready for review, you (or any interested party) can resurrect the patch entry by visiting https://commitfest.postgresql.org/38/2218/ and changing the status to "Needs Review", and then changing the status again to "Move to next CF". (Don't forget the second step; hopefully we will have streamlined this in the near future!) Thanks, --Jacob [1] https://postgr.es/m/flat/0ab66589-2f71-69b3-2002-49e821740...@timescale.com
Re: [PATCH] Implement INSERT SET syntax
Hello,
Here is a new version of the patch that applies to HEAD.
It also adds some regression tests for overriding {system,user} values
based on Wenjing Zeng's work.
Gareth
On Thu, 14 Jul 2022 at 22:40, Jacob Champion wrote:
>
> On Thu, Apr 7, 2022 at 11:29 AM Marko Tiikkaja wrote:
> > I can help with review and/or other work here. Please give me a
> > couple of weeks.
>
> Hi Marko, did you get a chance to pick up this patchset? If not, no
> worries; I can mark this RwF and we can try again in a future
> commitfest.
>
> Thanks,
> --Jacob
>
>
>
>
From f1ab7edadac846883b9c12f02ecc6c64dd293060 Mon Sep 17 00:00:00 2001
From: Gareth Palmer
Date: Thu, 14 Jul 2022 01:47:17 +
Subject: [PATCH] Implement INSERT SET syntax
Allow the target column and values of an INSERT statement to be specified
using a SET clause in the same manner as that of an UPDATE statement.
The advantage of using the INSERT SET style is that the columns and values
are kept together, which can make changing or removing a column or value
from a large list easier.
A simple example that uses SET instead of a VALUES() clause:
INSERT INTO t SET c1 = 'foo', c2 = 'bar', c3 = 'baz';
Values can also be sourced from other tables similar to the INSERT INTO
SELECT FROM syntax:
INSERT INTO t SET c1 = x.c1, c2 = x.c2 FROM x WHERE x.c2 > 10 LIMIT 10;
INSERT SET is not part of any SQL standard, however this syntax is also
implemented by MySQL. Their implementation does not support specifying a
FROM clause.
---
doc/src/sgml/ref/insert.sgml | 56 +-
src/backend/parser/gram.y | 77 ++-
src/test/regress/expected/identity.out| 42 ++
src/test/regress/expected/insert.out | 13 +++-
src/test/regress/expected/insert_conflict.out | 2 +
src/test/regress/expected/with.out| 20 +
src/test/regress/sql/identity.sql | 9 +++
src/test/regress/sql/insert.sql | 1 +
src/test/regress/sql/insert_conflict.sql | 3 +
src/test/regress/sql/with.sql | 9 +++
10 files changed, 210 insertions(+), 22 deletions(-)
diff --git a/doc/src/sgml/ref/insert.sgml b/doc/src/sgml/ref/insert.sgml
index a9af9959c0..b774b6ce74 100644
--- a/doc/src/sgml/ref/insert.sgml
+++ b/doc/src/sgml/ref/insert.sgml
@@ -28,6 +28,16 @@ INSERT INTO table_name [ AS conflict_target ] conflict_action ]
[ RETURNING * | output_expression [ [ AS ] output_name ] [, ...] ]
+
+[ WITH [ RECURSIVE ] with_query [, ...] ]
+INSERT INTO table_name [ AS alias ]
+[ OVERRIDING { SYSTEM | USER} VALUE ]
+SET { column_name = { expression | DEFAULT } } [, ...]
+[ FROM from_clause ]
+[ ON CONFLICT [ conflict_target ] conflict_action ]
+[ RETURNING * | output_expression [ [ AS ] output_name ] [, ...] ]
+
+
where conflict_target can be one of:
( { index_column_name | ( index_expression ) } [ COLLATE collation ] [ opclass ] [, ...] ) [ WHERE index_predicate ]
@@ -263,6 +273,18 @@ INSERT INTO table_name [ AS
+
+ from_clause
+
+
+A list of table expressions, allowing columns from other tables
+to be used as values in the expression.
+Refer to the statement for a
+description of the syntax.
+
+
+
+
DEFAULT
@@ -650,6 +672,15 @@ INSERT INTO films (code, title, did, date_prod, kind) VALUES
+
+Insert a row using SET syntax:
+
+
+INSERT INTO films SET code = 'MH832', title = 'Blade Runner',
+did = 201, date_prod = DEFAULT, kind = 'SciFi';
+
+
+
This example inserts some rows into table
films from a table tmp_films
@@ -696,6 +727,16 @@ WITH upd AS (
INSERT INTO employees_log SELECT *, current_timestamp FROM upd;
+
+ Insert multiple rows into employees_log containing
+the hours worked by each employee from time_sheets.
+
+INSERT INTO employees_log SET id = time_sheets.employee,
+total_hours = sum(time_sheets.hours) FROM time_sheets
+WHERE time_sheets.date '2019-11-15' GROUP BY time_sheets.employee;
+
+
+
Insert or update new distributors as appropriate. Assumes a unique
index has been defined that constrains values appearing in the
@@ -752,6 +793,18 @@ INSERT INTO distributors (did, dname) VALUES (9, 'Antwerp Design')
INSERT INTO distributors (did, dname) VALUES (10, 'Conrad International')
ON CONFLICT (did) WHERE is_active DO NOTHING;
+
+ Insert a new film into watched_films or increment the
+ number of times seen. Returns the new seen count, example assumes a
+ unique index has been defined that constrains the values appearing in
+ the title and year columns and
+ that seen_count defaults to 1.
+
+INSERT INTO watched_films SET title = 'Akira', year = 1988
+ ON CONFLICT (title, year) DO UPDATE SET seen_count = watched_films.seen_count + 1
+ RETURNING watched_films.seen_count;
+
+
@@ -762,7 +815,8 @@ INSERT INTO distributors (did, dname)
Re: [PATCH] Implement INSERT SET syntax
On Thu, Apr 7, 2022 at 11:29 AM Marko Tiikkaja wrote: > I can help with review and/or other work here. Please give me a > couple of weeks. Hi Marko, did you get a chance to pick up this patchset? If not, no worries; I can mark this RwF and we can try again in a future commitfest. Thanks, --Jacob
Re: [PATCH] Implement INSERT SET syntax
On Wed, Mar 23, 2022 at 5:33 PM Tom Lane wrote: > > Justin Pryzby writes: > > You have to either include the pre-requisite patches as 0001, and your > > patch as > > 0002 (as I'm doing now), or name your patch something other than *.diff or > > *.patch, so cfbot doesn't think it's a new version of the patch to be > > tested. > > This patch has been basically ignored for a full two years now. > (Remarkably, it's still passing in the cfbot.) > > I have to think that that means there's just not enough interest > to justify committing it. Should we mark it rejected and move on? > If not, what needs to happen to get it unstuck? I can help with review and/or other work here. Please give me a couple of weeks. .m
Re: [PATCH] Implement INSERT SET syntax
Justin Pryzby writes: > You have to either include the pre-requisite patches as 0001, and your patch > as > 0002 (as I'm doing now), or name your patch something other than *.diff or > *.patch, so cfbot doesn't think it's a new version of the patch to be tested. This patch has been basically ignored for a full two years now. (Remarkably, it's still passing in the cfbot.) I have to think that that means there's just not enough interest to justify committing it. Should we mark it rejected and move on? If not, what needs to happen to get it unstuck? regards, tom lane
Re: [PATCH] Implement INSERT SET syntax
(all fail) @@ -44,7 +46,8 @@ select * from inserttest; |5 | testing |5 | test |7 | testing -(4 rows) + |9 | testing +(5 rows) -- -- VALUES test @@ -58,10 +61,11 @@ select * from inserttest; |5 | testing |5 | test |7 | testing + |9 | testing 10 | 20 | 40 -1 |2 | testing 2 |3 | values are fun! -(7 rows) +(8 rows) -- -- TOASTed value test @@ -74,11 +78,12 @@ select col1, col2, char_length(col3) from inserttest; |5 | 7 |5 | 4 |7 | 7 + |9 | 7 10 | 20 | 2 -1 |2 | 7 2 |3 | 15 30 | 50 | 1 -(8 rows) +(9 rows) drop table inserttest; -- diff --git a/src/test/regress/expected/insert_conflict.out b/src/test/regress/expected/insert_conflict.out index 66d8633e3ec..0a342ca5e92 100644 --- a/src/test/regress/expected/insert_conflict.out +++ b/src/test/regress/expected/insert_conflict.out @@ -236,6 +236,8 @@ insert into insertconflicttest values (2, 'Orange') on conflict (key, key, key) insert into insertconflicttest values (1, 'Apple'), (2, 'Orange') on conflict (key) do update set (fruit, key) = (excluded.fruit, excluded.key); +-- Using insert set syntax +insert into insertconflicttest set key = 1, fruit = 'Banana' on conflict (key) do update set fruit = excluded.fruit; -- Give good diagnostic message when EXCLUDED.* spuriously referenced from -- RETURNING: insert into insertconflicttest values (1, 'Apple') on conflict (key) do update set fruit = excluded.fruit RETURNING excluded.fruit; diff --git a/src/test/regress/expected/with.out b/src/test/regress/expected/with.out index f15ece3bd18..e3a6e34d32a 100644 --- a/src/test/regress/expected/with.out +++ b/src/test/regress/expected/with.out @@ -1780,6 +1780,26 @@ SELECT * FROM y; 10 (10 rows) +TRUNCATE TABLE y; +WITH t AS ( +SELECT generate_series(1, 10) AS a +) +INSERT INTO y SET a = t.a+20 FROM t; +SELECT * FROM y; + a + + 21 + 22 + 23 + 24 + 25 + 26 + 27 + 28 + 29 + 30 +(10 rows) + DROP TABLE y; -- -- error cases diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql index bfaa8a3b277..1e4ecb37dbe 100644 --- a/src/test/regress/sql/insert.sql +++ b/src/test/regress/sql/insert.sql @@ -7,6 +7,7 @@ insert into inserttest (col2, col3) values (3, DEFAULT); insert into inserttest (col1, col2, col3) values (DEFAULT, 5, DEFAULT); insert into inserttest values (DEFAULT, 5, 'test'); insert into inserttest values (DEFAULT, 7); +insert into inserttest set col1 = DEFAULT, col2 = 9; select * from inserttest; diff --git a/src/test/regress/sql/insert_conflict.sql b/src/test/regress/sql/insert_conflict.sql index 23d5778b821..95223bd8313 100644 --- a/src/test/regress/sql/insert_conflict.sql +++ b/src/test/regress/sql/insert_conflict.sql @@ -97,6 +97,9 @@ insert into insertconflicttest values (1, 'Apple'), (2, 'Orange') on conflict (key) do update set (fruit, key) = (excluded.fruit, excluded.key); +-- Using insert set syntax +insert into insertconflicttest set key = 1, fruit = 'Banana' on conflict (key) do update set fruit = excluded.fruit; + -- Give good diagnostic message when EXCLUDED.* spuriously referenced from -- RETURNING: insert into insertconflicttest values (1, 'Apple') on conflict (key) do update set fruit = excluded.fruit RETURNING excluded.fruit; diff --git a/src/test/regress/sql/with.sql b/src/test/regress/sql/with.sql index 7ff9de97a5f..6ec7815e02b 100644 --- a/src/test/regress/sql/with.sql +++ b/src/test/regress/sql/with.sql @@ -787,6 +787,15 @@ DELETE FROM y USING t WHERE t.a = y.a RETURNING y.a; SELECT * FROM y; +TRUNCATE TABLE y; + +WITH t AS ( +SELECT generate_series(1, 10) AS a +) +INSERT INTO y SET a = t.a+20 FROM t; + +SELECT * FROM y; + DROP TABLE y; -- -- 2.17.1 >From 4c42f9fb4f4b596da9e1fc1b4d15009b6eb035a1 Mon Sep 17 00:00:00 2001 From: wenjing zeng Date: Fri, 21 Jan 2022 17:24:51 +0800 Subject: [PATCH 2/2] Since this feature adds INSERT OVERRIDING SET syntax, it is recommended to add some related testcases. Jan 21 wenjing zeng( 111) Re: [PATCH] Implement INSERT SET syntax --- src/test/regress/expected/identity.out | 22 ++ src/test/regress/sql/identity.sql | 15 +++ 2 files changed, 37 insertions(+) diff --git a/src/test/regress/expected/identity.out b/src/test/regress/expected/identity.out index 99811570b7b..5dd211e4563 100644 --- a/src/test/regress/expected/identity.out +++ b/src/test/regress/expected/identity.out @@ -160,6 +160,28 @@ SELECT * FROM itest5; 16 | iii (21 rows) +TRUNCATE TABLE itest5; +INSERT INTO itest5 OVERRIDING SYSTEM VALUE SET a = -1, b = 'aa'; +INSERT INTO itest5 OVERRIDING SYSTEM VALUE SET a = DEFAULT, b = 'bb'; +INSERT INTO itest5 OVERRIDING SYSTEM VALUE SET a = DEFAULT; +INSERT INTO itest5 OVERRIDING SYSTEM VALUE SET b = 'cc'; +INSERT INTO itest5 OVE
Re: [PATCH] Implement INSERT SET syntax
Since this feature adds INSERT OVERRIDING SET syntax, it is recommended to add some related testcases. Regards Wenjing > 2021年9月22日 07:38，Rachel Heaton 写道： > >> On 4/23/20 8:04 PM, Gareth Palmer wrote: >>> >>> Thank you for the review, attached is v7 of the patch which should >>> apply correcly to HEAD. >>> > > Hello Gareth, > > This patch no longer applies to HEAD, can you please submit a rebased version? > > Thanks, > Rachel > > 0001-insert-overriding-set-case.patch Description: Binary data
Re: [PATCH] Implement INSERT SET syntax
Hello Rachel,
On Wed, 22 Sept 2021 at 17:13, Rachel Heaton wrote:
>
> > On 4/23/20 8:04 PM, Gareth Palmer wrote:
> > >
> > > Thank you for the review, attached is v7 of the patch which should
> > > apply correcly to HEAD.
> > >
>
> Hello Gareth,
>
> This patch no longer applies to HEAD, can you please submit a rebased version?
Attached is a rebased version that should apply to HEAD.
Gareth
> Thanks,
> Rachel
>
>
>
>
From 7e222b4068e445a723f1692c5cdeec99d498a161 Mon Sep 17 00:00:00 2001
From: Gareth Palmer
Date: Wed, 22 Sep 2021 05:09:28 +
Subject: [PATCH] Implement INSERT SET syntax
Allow the target column and values of an INSERT statement to be specified
using a SET clause in the same manner as that of an UPDATE statement.
The advantage of using the INSERT SET style is that the columns and values
are kept together, which can make changing or removing a column or value
from a large list easier.
A simple example that uses SET instead of a VALUES() clause:
INSERT INTO t SET c1 = 'foo', c2 = 'bar', c3 = 'baz';
Values can also be sourced from other tables similar to the INSERT INTO
SELECT FROM syntax:
INSERT INTO t SET c1 = x.c1, c2 = x.c2 FROM x WHERE x.c2 > 10 LIMIT 10;
INSERT SET is not part of any SQL standard, however this syntax is also
implemented by MySQL. Their implementation does not support specifying a
FROM clause.
---
doc/src/sgml/ref/insert.sgml | 56 +-
src/backend/parser/gram.y | 77 ++-
src/test/regress/expected/insert.out | 13 +++-
src/test/regress/expected/insert_conflict.out | 2 +
src/test/regress/expected/with.out| 20 +
src/test/regress/sql/insert.sql | 1 +
src/test/regress/sql/insert_conflict.sql | 3 +
src/test/regress/sql/with.sql | 9 +++
8 files changed, 173 insertions(+), 8 deletions(-)
diff --git a/doc/src/sgml/ref/insert.sgml b/doc/src/sgml/ref/insert.sgml
index 2973b72b81..63c0579d4b 100644
--- a/doc/src/sgml/ref/insert.sgml
+++ b/doc/src/sgml/ref/insert.sgml
@@ -28,6 +28,16 @@ INSERT INTO table_name [ AS conflict_target ] conflict_action ]
[ RETURNING * | output_expression [ [ AS ] output_name ] [, ...] ]
+
+[ WITH [ RECURSIVE ] with_query [, ...] ]
+INSERT INTO table_name [ AS alias ]
+[ OVERRIDING { SYSTEM | USER} VALUE ]
+SET { column_name = { expression | DEFAULT } } [, ...]
+[ FROM from_clause ]
+[ ON CONFLICT [ conflict_target ] conflict_action ]
+[ RETURNING * | output_expression [ [ AS ] output_name ] [, ...] ]
+
+
where conflict_target can be one of:
( { index_column_name | ( index_expression ) } [ COLLATE collation ] [ opclass ] [, ...] ) [ WHERE index_predicate ]
@@ -263,6 +273,18 @@ INSERT INTO table_name [ AS
+
+ from_clause
+
+
+A list of table expressions, allowing columns from other tables
+to be used as values in the expression.
+Refer to the statement for a
+description of the syntax.
+
+
+
+
DEFAULT
@@ -643,6 +665,15 @@ INSERT INTO films (code, title, did, date_prod, kind) VALUES
+
+Insert a row using SET syntax:
+
+
+INSERT INTO films SET code = 'MH832', title = 'Blade Runner',
+did = 201, date_prod = DEFAULT, kind = 'SciFi';
+
+
+
This example inserts some rows into table
films from a table tmp_films
@@ -689,6 +720,16 @@ WITH upd AS (
INSERT INTO employees_log SELECT *, current_timestamp FROM upd;
+
+ Insert multiple rows into employees_log containing
+the hours worked by each employee from time_sheets.
+
+INSERT INTO employees_log SET id = time_sheets.employee,
+total_hours = sum(time_sheets.hours) FROM time_sheets
+WHERE time_sheets.date '2019-11-15' GROUP BY time_sheets.employee;
+
+
+
Insert or update new distributors as appropriate. Assumes a unique
index has been defined that constrains values appearing in the
@@ -745,6 +786,18 @@ INSERT INTO distributors (did, dname) VALUES (9, 'Antwerp Design')
INSERT INTO distributors (did, dname) VALUES (10, 'Conrad International')
ON CONFLICT (did) WHERE is_active DO NOTHING;
+
+ Insert a new film into watched_films or increment the
+ number of times seen. Returns the new seen count, example assumes a
+ unique index has been defined that constrains the values appearing in
+ the title and year columns and
+ that seen_count defaults to 1.
+
+INSERT INTO watched_films SET title = 'Akira', year = 1988
+ ON CONFLICT (title, year) DO UPDATE SET seen_count = watched_films.seen_count + 1
+ RETURNING watched_films.seen_count;
+
+
@@ -755,7 +808,8 @@ INSERT INTO distributors (did, dname) VALUES (10, 'Conrad International')
the RETURNING clause is a
PostgreSQL extension, as is the ability
to use WITH with INSERT, and the ability to
- specify an alternative action with ON CONFLICT.
+ specify an alternative action with ON
Re: [PATCH] Implement INSERT SET syntax
> On 4/23/20 8:04 PM, Gareth Palmer wrote: > > > > Thank you for the review, attached is v7 of the patch which should > > apply correcly to HEAD. > > Hello Gareth, This patch no longer applies to HEAD, can you please submit a rebased version? Thanks, Rachel
Re: [PATCH] Implement INSERT SET syntax
On 4/23/20 8:04 PM, Gareth Palmer wrote: Thank you for the review, attached is v7 of the patch which should apply correcly to HEAD. This version now uses it's own production rule for the SET clause to avoid the issue with MultiAssigmentRef nodes in the targetList. Ibrar, Movead, you are the reviewers of this patch. Do you think all of Tom's and Peter's concerns have been addressed? If so, please mark as Ready for Committer so somebody can have a look. If not, what remains to be done? Regards, -- -David da...@pgmasters.net
Re: [PATCH] Implement INSERT SET syntax
Hi Movead, > On 22/04/2020, at 2:40 PM, movead li wrote: > > The following review has been posted through the commitfest application: > make installcheck-world: tested, passed > Implements feature: tested, passed > Spec compliant: tested, passed > Documentation:tested, passed > > It builds failed by applying to the latest code version, and I try head > '73025140885c889410b9bfc4a30a3866396fc5db' which work well. > > The new status of this patch is: Waiting on Author Thank you for the review, attached is v7 of the patch which should apply correcly to HEAD. This version now uses it's own production rule for the SET clause to avoid the issue with MultiAssigmentRef nodes in the targetList. insert-set-v7.patch Description: Binary data
Re: [PATCH] Implement INSERT SET syntax
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: tested, passed Documentation:tested, passed It builds failed by applying to the latest code version, and I try head '73025140885c889410b9bfc4a30a3866396fc5db' which work well. The new status of this patch is: Waiting on Author
Re: [PATCH] Implement INSERT SET syntax
> On 26/03/2020, at 3:17 AM, Tom Lane wrote: > > Peter Eisentraut writes: >> On 2020-03-24 18:57, Tom Lane wrote: >>> 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? > >> What is the additional functionality that we are considering adding here? >> The thread started out proposing a more convenient syntax, but it seems >> to go deeper now and perhaps not everyone is following. > > AIUI, the proposal is to allow INSERT commands to be written > using an UPDATE-like syntax, for example > > INSERT INTO table SET col1 = value1, col2 = value2, ... [ FROM ... ] > > where everything after FROM is the same as it is in SELECT. My initial > belief was that this was strictly equivalent to what you could do with > a target-column-names list in standard INSERT, viz > > INSERT INTO table (col1, col2, ...) VALUES (value1, value2, ...); > or > INSERT INTO table (col1, col2, ...) SELECT value1, value2, ... FROM ... > > but it's arguably more legible/convenient because the column names > are written next to their values. > > However, that rewriting falls down for certain multiassignment cases > where you have a row source that can't be decomposed, such as my > example > > INSERT INTO table SET (col1, col2) = (SELECT value1, value2 FROM ...), > ... [ FROM ... ] > > So, just as we found for UPDATE, multiassignment syntax is strictly > stronger than plain column-by-column assignment. > > There are some secondary issues about which variants of this syntax > will allow a column value to be written as DEFAULT, and perhaps > about whether set-returning functions work. But the major point > right now is about whether its's possible to rewrite to standard > syntax. > > regards, tom lane Attached is v6 of the patch. As per the suggestion the SET clause list is checked for any MultiAssigmentRef nodes and to report an error if any are found. For example, the rule definition that previously caused a parser crash would now produce the following error: vagrant=> create rule r1 as on insert to foo do instead vagrant-> insert into bar set (f1,f2,f3) = (select f1,f2,f3 from foo); ERROR: INSERT SET syntax does not support multi-assignment of columns. LINE 2: insert into bar set (f1,f2,f3) = (select f1,f2,f3 from foo); ^ HINT: Specify the column assignments separately. Requiring a FROM clause was a way to differentiate between an INSERT with VALUES() which does allow DEFAULT and an INSERT with SELECT which does not. The idea was that it would help the user understand that they were writing a different type of query and that DEFAULT would not be allowed in that context. To show what it would look like without that requirement I have removed it from the v6 patch. In the first example works but the second one will generate an error. INSERT INTO t SET c1 = 1 WHERE true; INSERT INTO t SET c1 = DEFAULT WHERE true; insert-set-v6.patch Description: Binary data
Re: [PATCH] Implement INSERT SET syntax
Peter Eisentraut writes: > On 2020-03-24 18:57, Tom Lane wrote: >> 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? > What is the additional functionality that we are considering adding here? > The thread started out proposing a more convenient syntax, but it seems > to go deeper now and perhaps not everyone is following. AIUI, the proposal is to allow INSERT commands to be written using an UPDATE-like syntax, for example INSERT INTO table SET col1 = value1, col2 = value2, ... [ FROM ... ] where everything after FROM is the same as it is in SELECT. My initial belief was that this was strictly equivalent to what you could do with a target-column-names list in standard INSERT, viz INSERT INTO table (col1, col2, ...) VALUES (value1, value2, ...); or INSERT INTO table (col1, col2, ...) SELECT value1, value2, ... FROM ... but it's arguably more legible/convenient because the column names are written next to their values. However, that rewriting falls down for certain multiassignment cases where you have a row source that can't be decomposed, such as my example INSERT INTO table SET (col1, col2) = (SELECT value1, value2 FROM ...), ... [ FROM ... ] So, just as we found for UPDATE, multiassignment syntax is strictly stronger than plain column-by-column assignment. There are some secondary issues about which variants of this syntax will allow a column value to be written as DEFAULT, and perhaps about whether set-returning functions work. But the major point right now is about whether its's possible to rewrite to standard syntax. regards, tom lane
Re: [PATCH] Implement INSERT SET syntax
On 2020-03-24 18:57, Tom Lane wrote: 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? What is the additional functionality that we are considering adding here? The thread started out proposing a more convenient syntax, but it seems to go deeper now and perhaps not everyone is following. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: [PATCH] Implement INSERT SET syntax
I wrote: > 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? One compromise that might be worth thinking about is to disallow multiassignments in this syntax, so as to (1) avoid the possibility of generating something that can't be represented by standard INSERT and (2) get something done in time for v13. The end of March is not that far off. Perhaps somebody would come back and extend it later, or perhaps not. A slightly more ambitious compromise would be to allow multiassignment only when the source can be pulled apart into independent subexpressions, comparable to the restriction we used to have in UPDATE itself (before 8f889b108 or thereabouts). In either case the transformation could be done right in gram.y and a helpful error thrown for unsupported cases. regards, tom lane
Re: [PATCH] Implement INSERT SET syntax
David Steele 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
Re: [PATCH] Implement INSERT SET syntax
Hi Tom, On 12/3/19 4:44 AM, Gareth Palmer wrote: On Sun, Dec 1, 2019 at 4:32 PM Michael Paquier wrote: On Fri, Nov 22, 2019 at 12:24:15PM +1300, Gareth Palmer wrote: Attached is an updated patch with for_locking_clause added, test-cases re-use existing tables and the comments and documentation have been expanded. Per the automatic patch tester, documentation included in the patch does not build. Could you please fix that? I have moved the patch to next CF, waiting on author. Attached is a fixed version. Does this version of the patch address your concerns? Regards, -- -David da...@pgmasters.net
Re: [PATCH] Implement INSERT SET syntax
On Sun, Dec 1, 2019 at 4:32 PM Michael Paquier wrote:
>
> On Fri, Nov 22, 2019 at 12:24:15PM +1300, Gareth Palmer wrote:
> > Attached is an updated patch with for_locking_clause added, test-cases
> > re-use existing tables and the comments and documentation have been
> > expanded.
>
> Per the automatic patch tester, documentation included in the patch
> does not build. Could you please fix that? I have moved the patch to
> next CF, waiting on author.
Attached is a fixed version.
> --
> Michael
From c7c32435f0c0a1948e5c3ebd7d66f0bc415ee54e Mon Sep 17 00:00:00 2001
From: Gareth Palmer
Date: Mon, 2 Dec 2019 10:59:40 +
Subject: [PATCH] Implement INSERT SET syntax
Allow the target column and values of an INSERT statement to be specified
using a SET clause in the same manner as that of an UPDATE statement.
The advantage of using the INSERT SET style is that the columns and values
are kept together, which can make changing or removing a column or value
from a large list easier.
A simple example that uses SET instead of a VALUES() clause:
INSERT INTO t SET c1 = 'foo', c2 = 'bar', c3 = 'baz';
Values can also be sourced from other tables similar to the INSERT INTO
SELECT FROM syntax:
INSERT INTO t SET c1 = x.c1, c2 = x.c2 FROM x WHERE x.c2 > 10 LIMIT 10;
INSERT SET is not part of any SQL standard, however this syntax is also
implemented by MySQL. Their implementation does not support specifying a
FROM clause.
---
doc/src/sgml/ref/insert.sgml | 59 ++-
src/backend/parser/gram.y | 58 +-
src/backend/parser/parse_expr.c | 10 +++-
src/test/regress/expected/insert.out | 26 +---
src/test/regress/expected/insert_conflict.out | 2 +
src/test/regress/expected/with.out| 20 +++
src/test/regress/sql/insert.sql | 2 +
src/test/regress/sql/insert_conflict.sql | 3 +
src/test/regress/sql/with.sql | 9 +++
9 files changed, 177 insertions(+), 12 deletions(-)
diff --git a/doc/src/sgml/ref/insert.sgml b/doc/src/sgml/ref/insert.sgml
index e829c61642..b2f7c06f53 100644
--- a/doc/src/sgml/ref/insert.sgml
+++ b/doc/src/sgml/ref/insert.sgml
@@ -28,6 +28,19 @@ INSERT INTO table_name [ AS conflict_target ] conflict_action ]
[ RETURNING * | output_expression [ [ AS ] output_name ] [, ...] ]
+
+[ WITH [ RECURSIVE ] with_query [, ...] ]
+INSERT INTO table_name [ AS alias ]
+[ OVERRIDING { SYSTEM | USER} VALUE ]
+SET { column_name = { expression | DEFAULT } |
+ ( column_name [, ...] ) = [ ROW ] ( { expression | DEFAULT } [, ...] ) |
+ ( column_name [, ...] ) = ( sub-SELECT )
+} [, ...]
+[ FROM from_clause ]
+[ ON CONFLICT [ conflict_target ] conflict_action ]
+[ RETURNING * | output_expression [ [ AS ] output_name ] [, ...] ]
+
+
where conflict_target can be one of:
( { index_column_name | ( index_expression ) } [ COLLATE collation ] [ opclass ] [, ...] ) [ WHERE index_predicate ]
@@ -254,6 +267,18 @@ INSERT INTO table_name [ AS
+
+ from_clause
+
+
+A list of table expressions, allowing columns from other tables
+to be used as values in the expression.
+Refer to the statement for a
+description of the syntax.
+
+
+
+
DEFAULT
@@ -631,6 +656,15 @@ INSERT INTO films (code, title, did, date_prod, kind) VALUES
+
+Insert a row using SET syntax:
+
+
+INSERT INTO films SET code = 'MH832', title = 'Blade Runner',
+did = 201, date_prod = DEFAULT, kind = 'SciFi';
+
+
+
This example inserts some rows into table
films from a table tmp_films
@@ -677,6 +711,16 @@ WITH upd AS (
INSERT INTO employees_log SELECT *, current_timestamp FROM upd;
+
+ Insert multiple rows into employees_log containing
+the hours worked by each employee from time_sheets.
+
+INSERT INTO employees_log SET id = time_sheets.employee,
+total_hours = sum(time_sheets.hours) FROM time_sheets
+WHERE time_sheets.date '2019-11-15' GROUP BY time_sheets.employee;
+
+
+
Insert or update new distributors as appropriate. Assumes a unique
index has been defined that constrains values appearing in the
@@ -733,6 +777,18 @@ INSERT INTO distributors (did, dname) VALUES (9, 'Antwerp Design')
INSERT INTO distributors (did, dname) VALUES (10, 'Conrad International')
ON CONFLICT (did) WHERE is_active DO NOTHING;
+
+ Insert a new film into watched_films or increment the
+ number of times seen. Returns the new seen count, example assumes a
+ unique index has been defined that constrains the values appearing in
+ the title and year columns and
+ that seen_count defaults to 1.
+
+INSERT INTO watched_films SET title = 'Akira', year = 1988
+ ON CONFLICT (title, year) DO UPDATE SET seen_count = watched_films.seen_count + 1
+ RETURNING watched_films.seen_count;
+
+
@@ -743,7 +799,8
Re: [PATCH] Implement INSERT SET syntax
On Fri, Nov 22, 2019 at 12:24:15PM +1300, Gareth Palmer wrote: > Attached is an updated patch with for_locking_clause added, test-cases > re-use existing tables and the comments and documentation have been > expanded. Per the automatic patch tester, documentation included in the patch does not build. Could you please fix that? I have moved the patch to next CF, waiting on author. -- Michael signature.asc Description: PGP signature
Re: [PATCH] Implement INSERT SET syntax
> On 19/11/2019, at 5:05 PM, Gareth Palmer wrote: >> >> Since nobody has objected to this, I'm supposing that there's general >> consensus that that design sketch is OK, and we can move on to critiquing >> implementation details. I took a look, and didn't like much of what I saw. Attached is an updated patch with for_locking_clause added, test-cases re-use existing tables and the comments and documentation have been expanded. >> I'm setting this back to Waiting on Author. insert-set-v4.patch Description: Binary data
Re: [PATCH] Implement INSERT SET syntax
> On 15/11/2019, at 10:20 AM, Tom Lane wrote: > > Gareth Palmer writes: >>> On 19/08/2019, at 3:00 AM, Tom Lane wrote: >>> Perhaps the way to resolve Peter's objection is to make the syntax >>> more fully like UPDATE: >>>INSERT INTO target SET c1 = x, c2 = y+z, ... FROM tables-providing-x-y-z >>> (with the patch as-submitted corresponding to the case with an empty >>> FROM clause, hence no variables in the expressions-to-be-assigned). > >> Thanks for the feedback. Attached is version 3 of the patch that makes >> the syntax work more like an UPDATE statement when a FROM clause is used. > > Since nobody has objected to this, I'm supposing that there's general > consensus that that design sketch is OK, and we can move on to critiquing > implementation details. I took a look, and didn't like much of what I saw. > > * In the grammar, there's no real need to have separate productions > for the cases with FROM and without. The way you have it is awkward, > and it arbitrarily rejects combinations that work fine in plain > SELECT, such as WHERE without FROM. You should just do > > insert_set_clause: > SET set_clause_list from_clause where_clause > group_clause having_clause window_clause opt_sort_clause > opt_select_limit > > relying on the ability of all those symbols (except set_clause_list) to > reduce to empty. > > * This is randomly inconsistent with select_no_parens, and not in a > good way, because you've omitted the option that's actually most likely > to be useful, namely for_locking_clause. I wonder whether it's practical > to refactor select_no_parens so that the stuff involving optional trailing > clauses can be separated out into a production that insert_set_clause > could also use. Might not be worth the trouble, but I'm concerned > about select_no_parens growing additional clauses that we then forget > to also add to insert_set_clause. > > * I'm not sure if it's worth also refactoring simple_select so that > the "into_clause ... window_clause" business could be shared. But > it'd likely be a good idea to at least have a comment there noting > that any changes in that production might need to be applied to > insert_set_clause as well. > > * In kind of the same vein, it feels like the syntax documentation > is awkwardly failing to share commonality that it ought to be > able to share with the SELECT man page. > > * I dislike the random hacking you did in transformMultiAssignRef. > That weakens a useful check for error cases, and it's far from clear > why the new assertion is OK. It also raises the question of whether > this is really the only place you need to touch in parse analysis. > Perhaps it'd be better to consider inventing new EXPR_KIND_ values > for this situation; you'd then have to run around and look at all the > existing EXPR_KIND uses, but that seems like a useful cross-check > activity anyway. Or maybe we need to take two steps back and > understand why that change is needed at all. I'd imagined that this > patch would be only syntactic sugar for something you can do already, > so it's not quite clear to me why we need additional changes. > > (If it's *not* just syntactic sugar, then the scope of potential > problems becomes far greater, eg does ruleutils.c need to know > how to reconstruct a valid SQL command from a querytree like this. > If we're not touching ruleutils.c, we need to be sure that every > command that can be written this way can be written old-style.) So it appears as though it may not require any changes to ruleutils.c as the parser is converting the multi-assignments into separate columns, eg: CREATE RULE r1 AS ON INSERT TO tab1 DO INSTEAD INSERT INTO tab2 SET (col2, col1) = (new.col2, 0), col3 = tab3.col3 FROM tab3 The rule generated is: r1 AS ON INSERT TO tab1 DO INSTEAD INSERT INTO tab2 (col2, col1, col3) SELECT new.col2, 0 AS col1, tab3.col3 FROM tab3 It will trigger that Assert() though, as EXPR_KIND_SELECT_TARGET is now also being passed to transformMultiassignRef(). > * Other documentation gripes: the lone example seems insufficient, > and there needs to be an entry under COMPATIBILITY pointing out > that this is not per SQL spec. > > * Some of the test cases seem to be expensively repeating > construction/destruction of tables that they could have shared with > existing test cases. I do not consider it a virtue for new tests > added to an existing test script to be resolutely independent of > what's already in that script. > > I'm setting this back to Waiting on Author. > > regards, tom lane
Re: [PATCH] Implement INSERT SET syntax
> On 15/11/2019, at 10:20 AM, Tom Lane wrote: > > Gareth Palmer writes: >>> On 19/08/2019, at 3:00 AM, Tom Lane wrote: >>> Perhaps the way to resolve Peter's objection is to make the syntax >>> more fully like UPDATE: >>>INSERT INTO target SET c1 = x, c2 = y+z, ... FROM tables-providing-x-y-z >>> (with the patch as-submitted corresponding to the case with an empty >>> FROM clause, hence no variables in the expressions-to-be-assigned). > >> Thanks for the feedback. Attached is version 3 of the patch that makes >> the syntax work more like an UPDATE statement when a FROM clause is used. > > Since nobody has objected to this, I'm supposing that there's general > consensus that that design sketch is OK, and we can move on to critiquing > implementation details. I took a look, and didn't like much of what I saw. > > * In the grammar, there's no real need to have separate productions > for the cases with FROM and without. The way you have it is awkward, > and it arbitrarily rejects combinations that work fine in plain > SELECT, such as WHERE without FROM. You should just do > > insert_set_clause: > SET set_clause_list from_clause where_clause > group_clause having_clause window_clause opt_sort_clause > opt_select_limit > > relying on the ability of all those symbols (except set_clause_list) to > reduce to empty. There are two separate productions to match the two different types of inserts: INSERT with VALUES and INSERT with SELECT. The former has to store the the values in valuesLists so that DEFAULT can still be used. Allowing a WHERE without a FROM also mean that while this would work: INSERT INTO t SET c = DEFAULT; But this would fail with 'DEFAULT is not allowed in this context': INSERT INTO t SET c = DEFAULT WHERE true; I should have put a comment explaining why there are two rules. It could be combined into one production but there would have to be a check that $4 .. $9 are NULL to determine what type of INSERT to use. transformInsertStmt() also has an optimisation for the case of a single valueLists entry. > * This is randomly inconsistent with select_no_parens, and not in a > good way, because you've omitted the option that's actually most likely > to be useful, namely for_locking_clause. I wonder whether it's practical > to refactor select_no_parens so that the stuff involving optional trailing > clauses can be separated out into a production that insert_set_clause > could also use. Might not be worth the trouble, but I'm concerned > about select_no_parens growing additional clauses that we then forget > to also add to insert_set_clause. > > * I'm not sure if it's worth also refactoring simple_select so that > the "into_clause ... window_clause" business could be shared. But > it'd likely be a good idea to at least have a comment there noting > that any changes in that production might need to be applied to > insert_set_clause as well. I can add opt_for_locking_clause and a comment to simple_select to start with while the format of insert_set_clause is still being worked out. > * In kind of the same vein, it feels like the syntax documentation > is awkwardly failing to share commonality that it ought to be > able to share with the SELECT man page. I could collapse the from clause to just '[ FROM from_clause ]' and have it refer to the from clause and everything after it in SELECT. > * I dislike the random hacking you did in transformMultiAssignRef. > That weakens a useful check for error cases, and it's far from clear > why the new assertion is OK. It also raises the question of whether > this is really the only place you need to touch in parse analysis. > Perhaps it'd be better to consider inventing new EXPR_KIND_ values > for this situation; you'd then have to run around and look at all the > existing EXPR_KIND uses, but that seems like a useful cross-check > activity anyway. Or maybe we need to take two steps back and > understand why that change is needed at all. I'd imagined that this > patch would be only syntactic sugar for something you can do already, > so it's not quite clear to me why we need additional changes. > > (If it's *not* just syntactic sugar, then the scope of potential > problems becomes far greater, eg does ruleutils.c need to know > how to reconstruct a valid SQL command from a querytree like this. > If we're not touching ruleutils.c, we need to be sure that every > command that can be written this way can be written old-style.) It was intended to just be syntatic sugar, however because set_clause_list is being re-used the ability to do multi-assignment in an INSERT's targetList 'came along for the ride' which has no equivalent in the current INSERT syntax. That would be why those EXPR_KIND's are now appearing in transformMultiAssignRef(). There are 3 things that could be done here: 1. Update ruletutils.c to emit INSERT SET in get_insert_query_def() if query->hasSubLinks is true. 2.
Re: [PATCH] Implement INSERT SET syntax
Pantelis Theodosiou writes: > On 19/08/2019, at 3:00 AM, Tom Lane wrote: >>> Perhaps the way to resolve Peter's objection is to make the syntax >>> more fully like UPDATE: >>> INSERT INTO target SET c1 = x, c2 = y+z, ... FROM >>> tables-providing-x-y-z > Regarding syntax and considering that it makes INSERT look like UPDATE: > there is another difference between INSERT and UPDATE. INSERT allows SELECT > with ORDER BY and OFFSET/LIMIT (or FETCH FIRST), e.g.: ... > But UPDATE doesn't. I suppose the proposed behaviour of INSERT .. SET will > be the same as standard INSERT. So we'll need a note for the differences > between INSERT/SET and UPDATE/SET syntax. I was supposing that this syntax should be just another way to spell INSERT INTO target (columnlist) SELECT ... So everything past FROM would work exactly like it does in SELECT. > On a related not, column aliases can be used in ORDER BY, e.g: As proposed, there's no option equivalent to writing output-column aliases in the INSERT ... SELECT form, so the question doesn't come up. regards, tom lane
Re: [PATCH] Implement INSERT SET syntax
On Thu, Nov 14, 2019 at 9:20 PM Tom Lane wrote: > Gareth Palmer writes: > >> On 19/08/2019, at 3:00 AM, Tom Lane wrote: > >> Perhaps the way to resolve Peter's objection is to make the syntax > >> more fully like UPDATE: > >> INSERT INTO target SET c1 = x, c2 = y+z, ... FROM > tables-providing-x-y-z > >> (with the patch as-submitted corresponding to the case with an empty > >> FROM clause, hence no variables in the expressions-to-be-assigned). > > > Thanks for the feedback. Attached is version 3 of the patch that makes > > the syntax work more like an UPDATE statement when a FROM clause is used. > > Since nobody has objected to this, I'm supposing that there's general > consensus that that design sketch is OK, and we can move on to critiquing > implementation details. I took a look, and didn't like much of what I saw. > > ... > > I'm setting this back to Waiting on Author. > > regards, tom lane > > > Regarding syntax and considering that it makes INSERT look like UPDATE: there is another difference between INSERT and UPDATE. INSERT allows SELECT with ORDER BY and OFFSET/LIMIT (or FETCH FIRST), e.g.: INSERT INTO t (a,b) SELECT a+10. b+10 FROM t ORDER BY a LIMIT 3; But UPDATE doesn't. I suppose the proposed behaviour of INSERT .. SET will be the same as standard INSERT. So we'll need a note for the differences between INSERT/SET and UPDATE/SET syntax. On a related not, column aliases can be used in ORDER BY, e.g: insert into t (a, b) select a + 20, b - 2 * a as f from t order by f desc limit 3 ; Would that be expressed as follows?: insert into t set a = a + 20, b = b - 2 * a as f from t order by f desc limit 3 ; Best regards, Pantelis Theodosiou
Re: [PATCH] Implement INSERT SET syntax
Gareth Palmer writes: >> On 19/08/2019, at 3:00 AM, Tom Lane wrote: >> Perhaps the way to resolve Peter's objection is to make the syntax >> more fully like UPDATE: >> INSERT INTO target SET c1 = x, c2 = y+z, ... FROM tables-providing-x-y-z >> (with the patch as-submitted corresponding to the case with an empty >> FROM clause, hence no variables in the expressions-to-be-assigned). > Thanks for the feedback. Attached is version 3 of the patch that makes > the syntax work more like an UPDATE statement when a FROM clause is used. Since nobody has objected to this, I'm supposing that there's general consensus that that design sketch is OK, and we can move on to critiquing implementation details. I took a look, and didn't like much of what I saw. * In the grammar, there's no real need to have separate productions for the cases with FROM and without. The way you have it is awkward, and it arbitrarily rejects combinations that work fine in plain SELECT, such as WHERE without FROM. You should just do insert_set_clause: SET set_clause_list from_clause where_clause group_clause having_clause window_clause opt_sort_clause opt_select_limit relying on the ability of all those symbols (except set_clause_list) to reduce to empty. * This is randomly inconsistent with select_no_parens, and not in a good way, because you've omitted the option that's actually most likely to be useful, namely for_locking_clause. I wonder whether it's practical to refactor select_no_parens so that the stuff involving optional trailing clauses can be separated out into a production that insert_set_clause could also use. Might not be worth the trouble, but I'm concerned about select_no_parens growing additional clauses that we then forget to also add to insert_set_clause. * I'm not sure if it's worth also refactoring simple_select so that the "into_clause ... window_clause" business could be shared. But it'd likely be a good idea to at least have a comment there noting that any changes in that production might need to be applied to insert_set_clause as well. * In kind of the same vein, it feels like the syntax documentation is awkwardly failing to share commonality that it ought to be able to share with the SELECT man page. * I dislike the random hacking you did in transformMultiAssignRef. That weakens a useful check for error cases, and it's far from clear why the new assertion is OK. It also raises the question of whether this is really the only place you need to touch in parse analysis. Perhaps it'd be better to consider inventing new EXPR_KIND_ values for this situation; you'd then have to run around and look at all the existing EXPR_KIND uses, but that seems like a useful cross-check activity anyway. Or maybe we need to take two steps back and understand why that change is needed at all. I'd imagined that this patch would be only syntactic sugar for something you can do already, so it's not quite clear to me why we need additional changes. (If it's *not* just syntactic sugar, then the scope of potential problems becomes far greater, eg does ruleutils.c need to know how to reconstruct a valid SQL command from a querytree like this. If we're not touching ruleutils.c, we need to be sure that every command that can be written this way can be written old-style.) * Other documentation gripes: the lone example seems insufficient, and there needs to be an entry under COMPATIBILITY pointing out that this is not per SQL spec. * Some of the test cases seem to be expensively repeating construction/destruction of tables that they could have shared with existing test cases. I do not consider it a virtue for new tests added to an existing test script to be resolutely independent of what's already in that script. I'm setting this back to Waiting on Author. regards, tom lane
Re: [PATCH] Implement INSERT SET syntax
On Fri, Nov 1, 2019 at 6:31 PM Robert Haas wrote: > On Sun, Aug 18, 2019 at 11:00 AM Tom Lane wrote: > > Perhaps the way to resolve Peter's objection is to make the syntax > > more fully like UPDATE: > > > > INSERT INTO target SET c1 = x, c2 = y+z, ... FROM tables-providing-x-y-z > > > > (with the patch as-submitted corresponding to the case with an empty > > FROM clause, hence no variables in the expressions-to-be-assigned). > > > > Of course, this is not functionally distinct from > > > > INSERT INTO target(c1,c2,...) SELECT x, y+z, ... FROM > tables-providing-x-y-z > > > > and it's fair to question whether it's worth supporting a nonstandard > > syntax just to allow the target column names to be written closer to > > the expressions-to-be-assigned. > > For what it's worth, I think this would be useful enough to justify > its existence. Back in days of yore when dragons roamed the earth and > I wrote database-driven applications instead of hacking on the > database itself, I often wondered why I had to write two > completely-different looking SQL statements, one to insert the data > which a user had entered into a webform into the database, and another > to update previously-entered data. This feature would allow those > queries to be written in the same way, which would have pleased me, > back in the day. > I still do, and this would be a big help. I don't care if it's non-standard. .m
Re: [PATCH] Implement INSERT SET syntax
On Sun, Aug 18, 2019 at 11:00 AM Tom Lane wrote: > Perhaps the way to resolve Peter's objection is to make the syntax > more fully like UPDATE: > > INSERT INTO target SET c1 = x, c2 = y+z, ... FROM tables-providing-x-y-z > > (with the patch as-submitted corresponding to the case with an empty > FROM clause, hence no variables in the expressions-to-be-assigned). > > Of course, this is not functionally distinct from > > INSERT INTO target(c1,c2,...) SELECT x, y+z, ... FROM tables-providing-x-y-z > > and it's fair to question whether it's worth supporting a nonstandard > syntax just to allow the target column names to be written closer to > the expressions-to-be-assigned. For what it's worth, I think this would be useful enough to justify its existence. Back in days of yore when dragons roamed the earth and I wrote database-driven applications instead of hacking on the database itself, I often wondered why I had to write two completely-different looking SQL statements, one to insert the data which a user had entered into a webform into the database, and another to update previously-entered data. This feature would allow those queries to be written in the same way, which would have pleased me, back in the day. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: [PATCH] Implement INSERT SET syntax
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: tested, passed Documentation:not tested Patch looks to me and works on my machine 73025140885c889410b9bfc4a30a3866396fc5db - HEAD I have not reviewed the documentaion changes. The new status of this patch is: Ready for Committer
Re: [PATCH] Implement INSERT SET syntax
Hi Tom, > On 19/08/2019, at 3:00 AM, Tom Lane wrote: > > Peter Eisentraut writes: >> What I don't like about the syntax is that it kind of breaks the >> notional processing model of INSERT in a fundamental way. > > Agreed. I really don't like that this only works for a VALUES-like case > (and only the one-row form at that). It's hard to see it as anything > but a wart pasted onto the syntax. > >> Let's think about how we can achieve this using existing concepts in >> SQL. What we really need here at a fundamental level is an option to >> match $target to $table_source by column *name* rather than column >> *position*. There is existing syntax in SQL for that, namely >>a UNION b >> vs >>a UNION CORRESPONDING b > > A potential issue here --- and something that applies to Vik's question > as well, now that I think about it --- is that CORRESPONDING breaks down > in the face of ALTER TABLE RENAME COLUMN. Something that had been a > legal query before the rename might be invalid, or mean something quite > different, afterwards. This is really nasty for stored views/rules, > because we have neither a mechanism for forbidding input-table renames > nor a mechanism for revalidating views/rules afterwards. Maybe we could > make it go by resolving CORRESPONDING in the rewriter or planner, rather > than in parse analysis; but that seems quite unpleasant as well. > Changing our conclusions about the data types coming out of a UNION > really shouldn't happen later than parse analysis. > > The SET-style syntax doesn't have that problem, since it's explicit > about which values go into which columns. > > Perhaps the way to resolve Peter's objection is to make the syntax > more fully like UPDATE: > > INSERT INTO target SET c1 = x, c2 = y+z, ... FROM tables-providing-x-y-z > > (with the patch as-submitted corresponding to the case with an empty > FROM clause, hence no variables in the expressions-to-be-assigned). > > Of course, this is not functionally distinct from > > INSERT INTO target(c1,c2,...) SELECT x, y+z, ... FROM tables-providing-x-y-z > > and it's fair to question whether it's worth supporting a nonstandard > syntax just to allow the target column names to be written closer to > the expressions-to-be-assigned. Thanks for the feedback. Attached is version 3 of the patch that makes the syntax work more like an UPDATE statement when a FROM clause is used. So, an updated summary of the new syntax is: 1. Equivalent to VALUES(...): INSERT INTO t SET c1 = x, c2 = y, c3 = z; 2. Equivalent to INSERT INTO ... SELECT ...: INSERT INTO t SET c1 = sum(x.c1) FROM x WHERE x.c1 < y AND x.c2 != z GROUP BY x.c3 ORDER BY x.c4 ASC LIMIT a OFFSET b; Gareth > regards, tom lane insert-set-v3.patch Description: Binary data
Re: [PATCH] Implement INSERT SET syntax
Peter Eisentraut writes: > What I don't like about the syntax is that it kind of breaks the > notional processing model of INSERT in a fundamental way. Agreed. I really don't like that this only works for a VALUES-like case (and only the one-row form at that). It's hard to see it as anything but a wart pasted onto the syntax. > Let's think about how we can achieve this using existing concepts in > SQL. What we really need here at a fundamental level is an option to > match $target to $table_source by column *name* rather than column > *position*. There is existing syntax in SQL for that, namely > a UNION b > vs > a UNION CORRESPONDING b A potential issue here --- and something that applies to Vik's question as well, now that I think about it --- is that CORRESPONDING breaks down in the face of ALTER TABLE RENAME COLUMN. Something that had been a legal query before the rename might be invalid, or mean something quite different, afterwards. This is really nasty for stored views/rules, because we have neither a mechanism for forbidding input-table renames nor a mechanism for revalidating views/rules afterwards. Maybe we could make it go by resolving CORRESPONDING in the rewriter or planner, rather than in parse analysis; but that seems quite unpleasant as well. Changing our conclusions about the data types coming out of a UNION really shouldn't happen later than parse analysis. The SET-style syntax doesn't have that problem, since it's explicit about which values go into which columns. Perhaps the way to resolve Peter's objection is to make the syntax more fully like UPDATE: INSERT INTO target SET c1 = x, c2 = y+z, ... FROM tables-providing-x-y-z (with the patch as-submitted corresponding to the case with an empty FROM clause, hence no variables in the expressions-to-be-assigned). Of course, this is not functionally distinct from INSERT INTO target(c1,c2,...) SELECT x, y+z, ... FROM tables-providing-x-y-z and it's fair to question whether it's worth supporting a nonstandard syntax just to allow the target column names to be written closer to the expressions-to-be-assigned. regards, tom lane
Re: [PATCH] Implement INSERT SET syntax
Vik Fearing writes: > On 18/08/2019 11:03, Peter Eisentraut wrote: >> a UNION b >> vs >> a UNION CORRESPONDING b > I have a WIP patch for CORRESPONDING [BY]. Is there any interest in me > continuing it? If so, I'll start another thread for it. CORRESPONDING is in the SQL standard, so in theory we ought to provide it. I think the hard question is how big/complicated the patch would be --- if the answer is "complicated", maybe it's not worth it. People have submitted patches for it before that didn't go anywhere, suggesting that the tradeoffs are not very good ... but maybe you'll think of a better way. regards, tom lane
Re: [PATCH] Implement INSERT SET syntax
On 18/08/2019 11:03, Peter Eisentraut wrote: > > a UNION b > > vs > > a UNION CORRESPONDING b I have a WIP patch for CORRESPONDING [BY]. Is there any interest in me continuing it? If so, I'll start another thread for it. -- Vik Fearing
Re: [PATCH] Implement INSERT SET syntax
On 2019-08-16 05:19, Amit Kapila wrote: > I think this can be a handy feature in some cases as pointed by you, > but do we really want it for PostgreSQL? In the last round of > discussions as pointed by you, there doesn't seem to be a consensus > that we want this feature. I guess before spending too much time into > reviewing this feature, we should first build a consensus on whether > we need this. I think the problem this is attempting to solve is valid. What I don't like about the syntax is that it kind of breaks the notional processing model of INSERT in a fundamental way. The model is INSERT INTO $target $table_source where $table_source could be VALUES, SELECT, possibly others in theory. The proposed syntax changes this to only allow a single row to be specified via the SET syntax, and the SET syntax does not function as a row or table source in other contexts. Let's think about how we can achieve this using existing concepts in SQL. What we really need here at a fundamental level is an option to match $target to $table_source by column *name* rather than column *position*. There is existing syntax in SQL for that, namely a UNION b vs a UNION CORRESPONDING b I think this could be used for INSERT as well. And then you need a syntax to assign column names inside the VALUES rows. I think you could do either of the following: VALUES (a => 1, b => 2) or VALUES (1 AS a, 2 AS b) Another nice effect of this would be that you could so something like INSERT INTO tbl2 CORRESPONDING SELECT * FROM tbl1; which copies the contents of tbl1 to tbl2 if they have the same column names but allowing for a different column order. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: [PATCH] Implement INSERT SET syntax
On Fri, Aug 16, 2019 at 8:19 AM Amit Kapila wrote: > On Wed, Jul 17, 2019 at 10:00 AM Gareth Palmer > wrote: > > > > Hello, > > > > Attached is a patch that adds the option of using SET clause to specify > > the columns and values in an INSERT statement in the same manner as that > > of an UPDATE statement. > > > > A simple example that uses SET instead of a VALUES() clause: > > > > INSERT INTO t SET c1 = 'foo', c2 = 'bar', c3 = 'baz'; > > > > Values may also be sourced from a CTE using a FROM clause: > > > > WITH x AS ( > > SELECT 'foo' AS c1, 'bar' AS c2, 'baz' AS c3 > > ) > > INSERT INTO t SET c1 = x.c1, c2 = x.c2, c3 = x.c3 FROM x; > > > > The advantage of using the SET clause style is that the column and value > > are kept together, which can make changing or removing a column or value > from > > a large list easier. > > > > Internally the grammar parser converts INSERT SET without a FROM clause > into > > the equivalent INSERT with a VALUES clause. When using a FROM clause it > becomes > > the equivalent of INSERT with a SELECT statement. > > > > There was a brief discussion regarding INSERT SET on pgsql-hackers in > late > > August 2009 [1]. > > > > INSERT SET is not part of any SQL standard (that I am aware of), however > this > > syntax is also implemented by MySQL [2]. Their implementation does not > support > > specifying a FROM clause. > > > > I think this can be a handy feature in some cases as pointed by you, > but do we really want it for PostgreSQL? In the last round of > discussions as pointed by you, there doesn't seem to be a consensus > that we want this feature. I guess before spending too much time into > reviewing this feature, we should first build a consensus on whether > we need this. > > I agree with you Amit, that we need a consensus on that. Do we really need that feature or not. In the previous discussion, there was no resistance to have that in PostgreSQL, but some problem with the patch. Current patch is very simple and not invasive, but still, we need a consensus on that. Along with users, I request some senior hackers/committers to also > weigh in about the desirability of this feature. > > -- > With Regards, > Amit Kapila. > EnterpriseDB: http://www.enterprisedb.com > > > -- Ibrar Ahmed
Re: [PATCH] Implement INSERT SET syntax
On Wed, Jul 17, 2019 at 10:00 AM Gareth Palmer wrote: > > Hello, > > Attached is a patch that adds the option of using SET clause to specify > the columns and values in an INSERT statement in the same manner as that > of an UPDATE statement. > > A simple example that uses SET instead of a VALUES() clause: > > INSERT INTO t SET c1 = 'foo', c2 = 'bar', c3 = 'baz'; > > Values may also be sourced from a CTE using a FROM clause: > > WITH x AS ( > SELECT 'foo' AS c1, 'bar' AS c2, 'baz' AS c3 > ) > INSERT INTO t SET c1 = x.c1, c2 = x.c2, c3 = x.c3 FROM x; > > The advantage of using the SET clause style is that the column and value > are kept together, which can make changing or removing a column or value from > a large list easier. > > Internally the grammar parser converts INSERT SET without a FROM clause into > the equivalent INSERT with a VALUES clause. When using a FROM clause it > becomes > the equivalent of INSERT with a SELECT statement. > > There was a brief discussion regarding INSERT SET on pgsql-hackers in late > August 2009 [1]. > > INSERT SET is not part of any SQL standard (that I am aware of), however this > syntax is also implemented by MySQL [2]. Their implementation does not support > specifying a FROM clause. > I think this can be a handy feature in some cases as pointed by you, but do we really want it for PostgreSQL? In the last round of discussions as pointed by you, there doesn't seem to be a consensus that we want this feature. I guess before spending too much time into reviewing this feature, we should first build a consensus on whether we need this. Along with users, I request some senior hackers/committers to also weigh in about the desirability of this feature. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
Re: [PATCH] Implement INSERT SET syntax
Hi Ibrar, > On 16/08/2019, at 7:14 AM, Ibrar Ahmed wrote: > > Patch conflict with this assertion > Assert(pstate->p_expr_kind == EXPR_KIND_UPDATE_SOURCE); > > src/backend/parser/parse_expr.c line 1570 > > The new status of this patch is: Waiting on Author Thank-you for reviewing the patch. Attached is version 2 of the patch that fixes the above by allowing p_expr_kind to be EXPR_KIND_VALUES_SINGLE as well. Gareth insert-set-v2.patch Description: Binary data
Re: [PATCH] Implement INSERT SET syntax
Patch conflict with this assertion Assert(pstate->p_expr_kind == EXPR_KIND_UPDATE_SOURCE); src/backend/parser/parse_expr.c line 1570 The new status of this patch is: Waiting on Author
Re: [PATCH] Implement INSERT SET syntax
Hi Kyotaro,
Thank-you for looking at the patch.
> On 18/07/2019, at 6:54 PM, Kyotaro Horiguchi wrote:
>
> Hello.
>
> If I'm not missing something, "SELECT " without
> having FROM clause doesn't need to be tweaked. Thus
> insert_set_clause is useless and all we need here would be
> something like the following. (and the same for OVERRIDING.)
>
> + | SET set_clause_list from_clause
> + {
> + SelectStmt *n = makeNode(SelectStmt);
> + n->targetList = $2;
> + n->fromClause = $3;
> + $$ = makeNode(InsertStmt);
> + $$->selectStmt = (Node *)n;
> + $$->cols = $2;
> + }
While that would mostly work, it would prevent setting the column to its
default value using the DEFAULT keyword.
Only expressions specified in valuesLists allow DEFAULT to be used. Those
in targetList do not because transformInsertStmt() treats that as a general
SELECT statement and the grammar does not allow the use of DEFAULT there.
So this would generate a "DEFAULT is not allowed in this context" error
if only targetList was used:
INSERT INTO t set c1 = DEFAULT;
Regards,
Gareth
> regards.
>
> --
> Kyotaro Horiguchi
> NTT Open Source Software Center
Re: [PATCH] Implement INSERT SET syntax
Hello.
At Thu, 18 Jul 2019 11:30:04 +1200, Gareth Palmer
wrote in
> Hi Marko,
>
> > On 17/07/2019, at 5:52 PM, Marko Tiikkaja wrote:
> >
> > On Wed, Jul 17, 2019 at 7:30 AM Gareth Palmer
> > wrote:
> > Attached is a patch that adds the option of using SET clause to specify
> > the columns and values in an INSERT statement in the same manner as that
> > of an UPDATE statement.
> >
> > Cool! Thanks for working on this, I'd love to see the syntax in PG.
> >
> > There was a brief discussion regarding INSERT SET on pgsql-hackers in late
> > August 2009 [1].
> >
> > There was also at least one slightly more recent adventure:
> > https://www.postgresql.org/message-id/709e06c0-59c9-ccec-d216-21e38cb5ed61%40joh.to
> >
> > You might want to check that thread too, in case any of the criticism there
> > applies to this patch as well.
>
> Thank-you for the pointer to that thread.
>
> I think my version avoids issue raised there by doing the conversion of the
> SET clause as part of the INSERT grammar rules.
If I'm not missing something, "SELECT " without
having FROM clause doesn't need to be tweaked. Thus
insert_set_clause is useless and all we need here would be
something like the following. (and the same for OVERRIDING.)
+ | SET set_clause_list from_clause
+ {
+ SelectStmt *n = makeNode(SelectStmt);
+ n->targetList = $2;
+ n->fromClause = $3;
+ $$ = makeNode(InsertStmt);
+ $$->selectStmt = (Node *)n;
+ $$->cols = $2;
+ }
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
Re: [PATCH] Implement INSERT SET syntax
Hi Marko, > On 17/07/2019, at 5:52 PM, Marko Tiikkaja wrote: > > On Wed, Jul 17, 2019 at 7:30 AM Gareth Palmer > wrote: > Attached is a patch that adds the option of using SET clause to specify > the columns and values in an INSERT statement in the same manner as that > of an UPDATE statement. > > Cool! Thanks for working on this, I'd love to see the syntax in PG. > > There was a brief discussion regarding INSERT SET on pgsql-hackers in late > August 2009 [1]. > > There was also at least one slightly more recent adventure: > https://www.postgresql.org/message-id/709e06c0-59c9-ccec-d216-21e38cb5ed61%40joh.to > > You might want to check that thread too, in case any of the criticism there > applies to this patch as well. Thank-you for the pointer to that thread. I think my version avoids issue raised there by doing the conversion of the SET clause as part of the INSERT grammar rules. Gareth
Re: [PATCH] Implement INSERT SET syntax
On Wed, Jul 17, 2019 at 7:30 AM Gareth Palmer wrote: > Attached is a patch that adds the option of using SET clause to specify > the columns and values in an INSERT statement in the same manner as that > of an UPDATE statement. > Cool! Thanks for working on this, I'd love to see the syntax in PG. There was a brief discussion regarding INSERT SET on pgsql-hackers in late > August 2009 [1]. > There was also at least one slightly more recent adventure: https://www.postgresql.org/message-id/709e06c0-59c9-ccec-d216-21e38cb5ed61%40joh.to You might want to check that thread too, in case any of the criticism there applies to this patch as well. .m
