Re: [PATCH] Add support for INSERT ... SET syntax

From: Vaibhav Dalvi <vaibhav(dot)dalvi(at)enterprisedb(dot)com>
To: Suraj Kharage <suraj(dot)kharage(at)enterprisedb(dot)com>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: [PATCH] Add support for INSERT ... SET syntax
Date: 2026-09-02 12:39:40
Message-ID: CA+vB=AEbDGcAiJ54iu0arRje3Wnm++HUi4EmDQhXsgA6u-gaxQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

Thanks Suraj for the clarification. I see two issues here, both stemming
from the same root cause:

*Issue #1: A row that skips an array column entirely gets wrongly rejected*
Setup:
create table ta(id int primary key, arr int[] default array[1]);

Command:
insert into ta set (id=70, arr[1]=1), (id=71, arr[1]=2), (id=72);

Actual result:
ERROR: cannot set an array element to DEFAULT

What's wrong: row id=72 doesn't mention arr in any way, no slot, no whole
column, nothing.
In plain English, it's saying "I don't care about arr, just use whatever it
normally is."
That should give it the column's default ({1} here, since one was declared,
or NULL if not)
exactly like skipping any other ordinary column works fine. Instead,
because other rows in the
same statement touch arr[1], the code silently treats row 72's silence as
if it had explicitly written
arr[1] = DEFAULT, and setting a single array slot to DEFAULT is always that
asked for nothing
gets blamed for a request it never made, and the whole statement fails.

Expected result:
id | arr
----+-----
70 | {1}
71 | {2}
72 | {1} -- the column's default, same as ny other statement

*Issue #2: Mixing "whole column" and "one slot" across different rows gets
wrongly rejected*
Setup:
create table t1(id int primary key, arr int[] default array[1]);

Command:
insert into t1 set (id=1, arr[1]=1), (id=2, arr=default);

Actual result:
ERROR: column "arr" specified more than once

What's wrong: Row 1 wants to set slot 1 of its own array. Row 2 wants its
own array to just be the default.
These are two completely separate rows with two completely separate array
values, nothing about them
actually conflicts. However, the code doesn't check rows independently; it
first lumps every row's arr-related
target into one shared bucket, and only then checks for conflicts. In that
shared bucket, "the whole column"
and "just slot 1" look like a contradiction, even though they aren't, since
they belong to two different rows, not
one row saying two contradictory things about the same value.

Expected result:
id | arr
----+-----
1 | {1}
2 | {1} -- the column's default

Another thought here is: should we restrict array/field indirection targets
instead like below?

insert into t1 set (id=1, arr[1]=1), (id=2, arr=default);
ERROR: array/field indirection targets are not supported in multi-row
INSERT ... SET

Per my knowledge, MySQL and MariaDB support this INSERT ... SET syntax:
- Standard MySQL does not support multi-row INSERT ... SET directly using SET
... SET.
In MySQL, INSERT ... SET is strictly single-row:

INSERT INTO employees SET id = 1, name = 'Alice', salary = 70000;

- MariaDB extends the standard INSERT ... SET syntax to support multiple
rows by repeating the SET clause for each row:

INSERT INTO employees
SET id = 1, name = 'Alice', salary = 70000,
SET id = 2, name = 'Bob', salary = 80000,
SET id = 3, name = 'Charlie', salary = 75000;

The point is to check if we can think in that direction?

Regards,
Vaibhav

On Mon, Aug 31, 2026 at 10:20 AM Suraj Kharage <
suraj(dot)kharage(at)enterprisedb(dot)com> wrote:

>
> On Fri, Aug 28, 2026 at 11:39 AM Vaibhav Dalvi <
> vaibhav(dot)dalvi(at)enterprisedb(dot)com> wrote:
>
>> create table ta(id int primary key, arr int[] default array[1]);
>>
>> -- classic VALUES: row 3 gets the table's default {1}
>> insert into ta values (70, array[1]), (71, array[2]), (72, default);
>> -- id | arr
>> -- ----+-----
>> -- 70 | {1}
>> -- 71 | {2}
>> -- 72 | {1}
>>
>> -- new SET syntax: same intent, row 3 just omits arr
>> insert into ta set (id=170, arr[1]=1), (id=171, arr[1]=2), (id=172);
>> -- ERROR: cannot set an array element to DEFAULT
>>
>
> Thanks for reporting this. I have looked into this and feel that this is
> an existing behavior for INSERT command.
> transformAssignedExpr() unconditionally rejects DEFAULT for any
> indirection target (array element or subfield).
>
> For e.g. for plain insert as well it will fail -
> postgres(at)2495950=#INSERT INTO ta(id, arr[1]) VALUES (70,1),(71,2),(72,
> default);
> ERROR: cannot set an array element to DEFAULT
> LINE 1: INSERT INTO ta(id, arr[1]) VALUES (70,1),(71,2),(72, default...
>
>
> It will work in both cases, if we try to insert in arr column.
> postgres(at)2495950=#INSERT INTO ta VALUES (70,array[1]),(71,array[2]),(72,
> default);
> INSERT 0 3
> postgres(at)2495950=#select * from ta;
> id | arr
> ----+-----
> 70 | {1}
> 71 | {2}
> 72 | {1}
> (3 rows)
>
>
> postgres(at)2495950=#insert into ta set (id=170, arr=array[1]), (id=171,
> arr=array[2]), (id=172, arr=default);
> INSERT 0 3
> postgres(at)2495950=#select * from ta;
> id | arr
> -----+-----
> 170 | {1}
> 171 | {2}
> 172 | {1}
> (3 rows)
>
> I hope this helps.
>
> Regards,
> Suraj
>
>
>>
>> *Classic VALUES: *if a row skips a column (or writes DEFAULT), Postgres
>> asks, "What's the default for the whole column arr?" —
>> table says {1}, done.
>>
>> *New INSERT ... SET:* if a row skips arr, but other rows in the same
>> statement use arr[1]=..., Postgres instead asks "what's
>> the default for just slot 1 of arr?" — and that question has always been
>> illegal in Postgres (you can't default one array
>> slot). So it errors, even though the row never asked for a slot at all —
>> it just wanted the whole column left alone.
>>
>> *Why is this a bug?: *row 172 never used array-indirection syntax at
>> all. it should get the plain, legal,
>> whole-column default ({1}), exactly like classic VALUES gives it.
>> Instead, the code wrongly forces it through the "set array
>> slot 1 to default" path just because other rows in the same statement
>> happened to use arr[1]. That path is always illegal
>> in Postgres, so a perfectly valid statement fails with an error the
>> user's SQL never requested.
>>
>> Regards,
>> Vaibhav
>>
>> On Thu, Aug 27, 2026 at 4:25 PM Suraj Kharage <
>> suraj(dot)kharage(at)enterprisedb(dot)com> wrote:
>>
>>> Thank you Vaibhav for the review.
>>>
>>> I have fixed these issues in the attached v4 patch. Please have a look.
>>> --
>>>
>>> Thanks & Regards,
>>> Suraj kharage,
>>>
>>>
>>>
>>> enterprisedb.com <https://www.enterprisedb.com/>
>>>
>>>
>>> On Wed, Aug 26, 2026 at 6:32 PM Vaibhav Dalvi <
>>> vaibhav(dot)dalvi(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi Suraj,
>>>>
>>>> I have a few observations regarding the latest v4 patch:
>>>>
>>>> 1. Assigning two different subfields or elements of the same column in
>>>> a single row is rejected,
>>>> even though the equivalent column-list INSERT syntax accepts it:
>>>>
>>>> postgres=# create type comp_t as (x int, y int);
>>>> CREATE TYPE
>>>> postgres=# create table t2 (id int primary key, c comp_t);
>>>> CREATE TABLE
>>>> postgres=# insert into t2 (id, c.x, c.y) values (1, 5, 6);
>>>> INSERT 0 1
>>>> postgres=# insert into t2 set id=2, c.x=7, c.y=8;
>>>> ERROR: column "c" specified more than once
>>>> LINE 1: insert into t2 set id=2, c.x=7, c.y=8;
>>>> ^
>>>>
>>>> The same failure occurs with an array column, without requiring a
>>>> custom type:
>>>>
>>>> postgres=# create table t3 (id int primary key, arr int[]);
>>>> CREATE TABLE
>>>> postgres=# insert into t3 (id, arr[1], arr[2]) values (1, 10, 20);
>>>> INSERT 0 1
>>>> postgres=# insert into t3 set id=2, arr[1]=30, arr[2]=40;
>>>> ERROR: column "arr" specified more than once
>>>> LINE 1: insert into t3 set id=2, arr[1]=30, arr[2]=40;
>>>> ^
>>>>
>>>> 2. There is a silent misassignment across rows in multi-row SET syntax:
>>>>
>>>> postgres=# create table t7 (id int primary key, arr int[]);
>>>> CREATE TABLE
>>>> postgres=# insert into t7 set (id=1, arr[1]=111), (id=2, arr[2]=222);
>>>> INSERT 0 2
>>>> postgres=# select * from t7;
>>>> id | arr
>>>> ----+-------
>>>> 1 | {111}
>>>> 2 | {222}
>>>> (2 rows)
>>>>
>>>> Although row 2 explicitly specifies arr[2]=222, the code only tracks
>>>> columns by name and
>>>> not by the specific element or field targeted. It retains the tracking
>>>> from row 1 ("arr → index [1]")
>>>> and applies it to subsequent rows. As a result, the value for row 2
>>>> silently lands in arr[1] instead
>>>> of arr[2], leaving arr[2] as NULL without throwing an error or warning.
>>>>
>>>> This differs from the standard VALUES limitation (e.g., INSERT INTO t7
>>>> (id, arr[1]) VALUES (1,111),(2,222)),
>>>> where applying arr[1] to both rows is expected because it is defined
>>>> once in the shared header.
>>>> In this multi-row SET case, the explicit per-row target is ignored and
>>>> silently corrupted rather than being rejected as unsupported.
>>>>
>>>> Regards,
>>>> Vaibhav
>>>>
>>>>
>>>> On Tue, Aug 25, 2026 at 8:40 PM Mario González <gonzalemario(at)gmail(dot)com>
>>>> wrote:
>>>>
>>>>> On Tue, 14 Jul 2026 at 00:39, Suraj Kharage <
>>>>> suraj(dot)kharage(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> Thanks Mario for the review.
>>>>>>
>>>>>> On Mon, Jul 13, 2026 at 12:18 AM Mario González Troncoso <
>>>>>> gonzalemario(at)gmail(dot)com> wrote:
>>>>>>
>>>>>>> diff --git a/src/backend/nodes/nodeFuncs.c
>>>>>>> b/src/backend/nodes/nodeFuncs.c
>>>>>>> index 2a2e00b372e..11cb4fcd2da 100644
>>>>>>> --- a/src/backend/nodes/nodeFuncs.c
>>>>>>> +++ b/src/backend/nodes/nodeFuncs.c
>>>>>>> @@ -4370,6 +4370,8 @@ raw_expression_tree_walker_impl(Node *node,
>>>>>>> return true;
>>>>>>> if (WALK(stmt->selectStmt))
>>>>>>> return true;
>>>>>>> + if (WALK(stmt->setClauseList))
>>>>>>> + return true;
>>>>>>> if (WALK(stmt->onConflictClause))
>>>>>>>
>>>>>>> you used `stmt->setClauseList` however, I read the entire `raw_expression_tree_walker_impl`
>>>>>>> function and it seems we don't mix "clause" with "List" in the variable
>>>>>>> names. Reading the whole file, I just found "targetList" and "valuesList".
>>>>>>>
>>>>>>> If you get my point, maybe you could use "setClause" only? I know
>>>>>>> that sounds like something that exists in setter/getters stuff. Like we're
>>>>>>> setting a clause up but would it be worth looking for a new variable name?
>>>>>>> I personally think so. Actually, after reading `src/include/nodes/parsenodes.h`,
>>>>>>> I think we should go for a change.
>>>>>>>
>>>>>>
>>>>>> Renamed setClauseList as per your suggestion.
>>>>>>
>>>>>>
>>>>>>> ----
>>>>>>> Also, in src/backend/parser/analyze.c we can change a lot of those
>>>>>>> foreach by foreach_node, however, I need to ask, did you have a reason to
>>>>>>> not use foreach_node() when you first wrote the code? Maybe I'm missing
>>>>>>> something. Because this patch is on a commitfest already, I didn't want to
>>>>>>> send a patch we might need to squash if I'm right afterwards. That's why
>>>>>>> I'd like to show you what I did:
>>>>>>> https://github.com/postgres/postgres/commit/7be0538f2a5d916f2fb4a39764985b358cf6d379
>>>>>>> If you like I could send a v4- with the squashed version.
>>>>>>>
>>>>>>> diff --git a/src/backend/parser/analyze.c
>>>>>>> b/src/backend/parser/analyze.c
>>>>>>> index 70c75d0bb20..d2f5b0edcc8 100644
>>>>>>> --- a/src/backend/parser/analyze.c
>>>>>>> +++ b/src/backend/parser/analyze.c
>>>>>>> @@ -679,31 +679,23 @@ transformInsertSetClause(ParseState *pstate,
>>>>>>> List *setClauseList,
>>>>>>> {
>>>>>>> List *all_cols = NIL; /* List of all
>>>>>>> unique column names */
>>>>>>> List *valuesLists = NIL;
>>>>>>> - ListCell *outer_lc;
>>>>>>> - ListCell *lc;
>>>>>>>
>>>>>>> /*
>>>>>>> * First pass: collect all unique column names from all rows.
>>>>>>> * We need to scan all rows first to determine the complete
>>>>>>> set of columns.
>>>>>>> * Also check for duplicate columns within each row.
>>>>>>> */
>>>>>>> - foreach(outer_lc, setClauseList)
>>>>>>> + foreach_node(List, set_clause, setClauseList)
>>>>>>> {
>>>>>>> - List *set_clause = (List *) lfirst(outer_lc);
>>>>>>> List *row_cols = NIL; /* Columns
>>>>>>> seen in this row */
>>>>>>> - ListCell *set_lc;
>>>>>>> [...]
>>>>>>>
>>>>>>
>>>>>> Used foreach_node as per your suggestion.
>>>>>>
>>>>>> I have addressed your review comments in the attached v4 patch.
>>>>>>
>>>>>>
>>>>> lgtm Suraj. I hope you can find a committer that buys you with this
>>>>> idea
>>>>>
>>>>>
>>>>> --
>>>>> Mario Gonzalez
>>>>>
>>>>

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Floris Van Nee 2026-09-02 12:50:18 Re: OOM kill during planning - pg_terminate_backend is ignored
Previous Message Ayoub Kazar 2026-09-02 12:38:13 Re: Add pg_stat_vfdcache view for VFD cache statistics