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

Hi Suraj,

Thanks for working on this.

I think the following bug also needs to be fixed.

1. Example:
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

*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
>>>
>>