Hi Suraj, This makes sense to me.
Regards, Vaibhav On Thu, Sep 10, 2026 at 2:58 PM Suraj Kharage < [email protected]> wrote: > > On Wed, Sep 2, 2026 at 6:10 PM Vaibhav Dalvi < > [email protected]> wrote: > >> 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? >> > Thanks for looking into this. > > Both issues share the same root cause: multi-row SET merges all rows into > one shared column list, > so once a column uses arr[1]-style indirection, every row must supply that > exact same target. > I think if we make this work, then need bypassing core Postgres > INSERT/rewrite internals logic and it becomes complex. > Postgres builds one assignment expression per column and reuses it for > every row (via Vars into the VALUES RTE), > and the rewriter refuses to merge a whole-column value with an > element/field assignment on the same column > ("multiple assignments to same column" in process_matched_tle()). > > So I went with your suggested restriction, but narrower: if all rows set > the same array/field target (e.g. all rows set arr[1]), > it keeps working as before. But if rows disagree — one row skips it, or > sets the whole column, or targets a different element/field — > we now raise one clear error. > Updated docs and tests accordingly. > > Please let me know your thoughts on the same. > > Regards, > Suraj > > > >> 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 < >> [email protected]> wrote: >> >>> >>> On Fri, Aug 28, 2026 at 11:39 AM Vaibhav Dalvi < >>> [email protected]> 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@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@2495950=#INSERT INTO ta VALUES >>> (70,array[1]),(71,array[2]),(72, default); >>> INSERT 0 3 >>> postgres@2495950=#select * from ta; >>> id | arr >>> ----+----- >>> 70 | {1} >>> 71 | {2} >>> 72 | {1} >>> (3 rows) >>> >>> >>> postgres@2495950=#insert into ta set (id=170, arr=array[1]), (id=171, >>> arr=array[2]), (id=172, arr=default); >>> INSERT 0 3 >>> postgres@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 < >>>> [email protected]> 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 < >>>>> [email protected]> 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 < >>>>>> [email protected]> wrote: >>>>>> >>>>>>> On Tue, 14 Jul 2026 at 00:39, Suraj Kharage < >>>>>>> [email protected]> wrote: >>>>>>> >>>>>>>> Thanks Mario for the review. >>>>>>>> >>>>>>>> On Mon, Jul 13, 2026 at 12:18 AM Mario González Troncoso < >>>>>>>> [email protected]> 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 >>>>>>> >>>>>>
