[PATCH] Implement INSERT SET syntax

On Wed, Jul 17, 2019 at 7:30 AM Gareth Palmer <gareth@internetnz.net.nz>
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:
/messages/by-id/709e06c0-59c9-ccec-d216-21e38cb5ed61@joh.to

You might want to check that thread too, in case any of the criticism there
applies to this patch as well.

.m

Hi Marko,

On 17/07/2019, at 5:52 PM, Marko Tiikkaja <marko@joh.to> wrote:

On Wed, Jul 17, 2019 at 7:30 AM Gareth Palmer <gareth@internetnz.net.nz> 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: /messages/by-id/709e06c0-59c9-ccec-d216-21e38cb5ed61@joh.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

Hello.

At Thu, 18 Jul 2019 11:30:04 +1200, Gareth Palmer <gareth@internetnz.net.nz> wrote in <D50A93EB-11F3-4ED2-8192-0328DF901BBA@internetnz.net.nz>

Hi Marko,

On 17/07/2019, at 5:52 PM, Marko Tiikkaja <marko@joh.to> wrote:

On Wed, Jul 17, 2019 at 7:30 AM Gareth Palmer <gareth@internetnz.net.nz> 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: /messages/by-id/709e06c0-59c9-ccec-d216-21e38cb5ed61@joh.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 <targetlist>" 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

Hi Kyotaro,

Thank-you for looking at the patch.

On 18/07/2019, at 6:54 PM, Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote:

Hello.

If I'm not missing something, "SELECT <targetlist>" 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

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

Hi Ibrar,

On 16/08/2019, at 7:14 AM, Ibrar Ahmed <ibrar.ahmad@gmail.com> 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

On Wed, Jul 17, 2019 at 10:00 AM Gareth Palmer <gareth@internetnz.net.nz> 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

On Fri, Aug 16, 2019 at 8:19 AM Amit Kapila <amit.kapila16@gmail.com> wrote:

On Wed, Jul 17, 2019 at 10:00 AM Gareth Palmer <gareth@internetnz.net.nz>
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

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

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

Vik Fearing <vik.fearing@2ndquadrant.com> 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

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> 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

Hi Tom,

On 19/08/2019, at 3:00 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> 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

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

On Sun, Aug 18, 2019 at 11:00 AM Tom Lane <tgl@sss.pgh.pa.us> 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

On Fri, Nov 1, 2019 at 6:31 PM Robert Haas <robertmhaas@gmail.com> wrote:

On Sun, Aug 18, 2019 at 11:00 AM Tom Lane <tgl@sss.pgh.pa.us> 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

Gareth Palmer <gareth@internetnz.net.nz> writes:

On 19/08/2019, at 3:00 AM, Tom Lane <tgl@sss.pgh.pa.us> 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

On Thu, Nov 14, 2019 at 9:20 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Gareth Palmer <gareth@internetnz.net.nz> writes:

On 19/08/2019, at 3:00 AM, Tom Lane <tgl@sss.pgh.pa.us> 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

Pantelis Theodosiou <ypercube@gmail.com> writes:

On 19/08/2019, at 3:00 AM, Tom Lane <tgl@sss.pgh.pa.us> 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