Обсуждение: Re: Allow ON CONFLICT DO UPDATE to return EXCLUDED values
I’ve looked through this patch. As far as I can tell, everything looks good, working and well commented.
The only nitpick I could find is a mispelling "EXLCUDED" → "EXCLUDED" in src/test/regress/expected/returning.out:464.
A maybe bigger question, is it nice that EXCLUDED is null when no conflict occurred? I can see the logic, but I think ergonomics wise it’d be nicer to have the proposed values in EXCLUDED, no matter what happened later. Then one can check EXCLUDED.value = NEW.value to see if one’s changes were added, for example.
The only nitpick I could find is a mispelling "EXLCUDED" → "EXCLUDED" in src/test/regress/expected/returning.out:464.
A maybe bigger question, is it nice that EXCLUDED is null when no conflict occurred? I can see the logic, but I think ergonomics wise it’d be nicer to have the proposed values in EXCLUDED, no matter what happened later. Then one can check EXCLUDED.value = NEW.value to see if one’s changes were added, for example.
/Viktor
On 7 Oct 2025 at 15:43 +0200, Dean Rasheed <dean.a.rasheed@gmail.com>, wrote:
Rebased version attached, following 904f6a593a0.
Regards,
Dean
On Tue, 7 Oct 2025 at 14:56, Viktor Holmberg <v@viktorh.net> wrote: > > I’ve looked through this patch. As far as I can tell, everything looks good, working and well commented. > The only nitpick I could find is a mispelling "EXLCUDED" → "EXCLUDED" in src/test/regress/expected/returning.out:464. Thanks for looking. I'm also glad to see that you picked up the INSERT ... ON CONFLICT DO SELECT patch, because I think these 2 features should work well together. I'll take another look at that one, but I'm not going to have any time this week. > A maybe bigger question, is it nice that EXCLUDED is null when no conflict occurred? I can see the logic, but I think ergonomicswise it’d be nicer to have the proposed values in EXCLUDED, no matter what happened later. Then one can check EXCLUDED.value= NEW.value to see if one’s changes were added, for example. Hmm, I'm not sure. I think it would be counter-intuitive to have non-null EXCLUDED values for rows that weren't excluded, and I think it's just as easy to check what values were added either way. Regards, Dean
On 07/10/2025 23:52, Dean Rasheed wrote: > On Tue, 7 Oct 2025 at 14:56, Viktor Holmberg <v@viktorh.net> wrote: >> A maybe bigger question, is it nice that EXCLUDED is null when no conflict occurred? I can see the logic, but I thinkergonomics wise it’d be nicer to have the proposed values in EXCLUDED, no matter what happened later. Then one can checkEXCLUDED.value = NEW.value to see if one’s changes were added, for example. > Hmm, I'm not sure. I think it would be counter-intuitive to have > non-null EXCLUDED values for rows that weren't excluded, and I think > it's just as easy to check what values were added either way. Agreed. EXCLUDED should be null or even inaccessible if the row wasn't excluded. -- Vik Fearing
Rebased version attached (now also works with ON CONFLICT DO SELECT). Regards, Dean
Вложения
On Thu, 12 Feb 2026 at 11:11, Dean Rasheed <dean.a.rasheed@gmail.com> wrote: > > Rebased version attached (now also works with ON CONFLICT DO SELECT). > Something else to consider is the fact that this patch allows both colname and EXCLUDED.colname to appear in the RETURNING list, where the former refers to the target relation as it always has done. We're pretty-much forced into that position, in order that people don't have to rewrite all their existing queries (the same was true when support for OLD/NEW was added to RETURNING). But that then leads to the odd situation where colname has to be qualified when it appears on the RHS of a SET clause, or in the WHERE clause of INSERT ON CONFLICT, but not when it appears in the RETURNING list. There was a separate discussion [1] about whether we should remove this requirement for colname to be qualified elsewhere in INSERT ON CONFLICT, and I think this patch adds more weight to that argument. [1] https://www.postgresql.org/message-id/flat/a31a76b6-b176-47fe-8778-9dfece231341@wikimedia.org So should we do that, and allow unqualified column names in ON CONFLICT SET and WHERE? Regards, Dean
On 12 Feb 2026 at 13:23 +0100, Dean Rasheed <dean.a.rasheed@gmail.com>, wrote:
On Thu, 12 Feb 2026 at 11:11, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
Rebased version attached (now also works with ON CONFLICT DO SELECT).
Something else to consider is the fact that this patch allows both
colname and EXCLUDED.colname to appear in the RETURNING list, where
the former refers to the target relation as it always has done. We're
pretty-much forced into that position, in order that people don't have
to rewrite all their existing queries (the same was true when support
for OLD/NEW was added to RETURNING).
But that then leads to the odd situation where colname has to be
qualified when it appears on the RHS of a SET clause, or in the WHERE
clause of INSERT ON CONFLICT, but not when it appears in the RETURNING
list.
There was a separate discussion [1] about whether we should remove
this requirement for colname to be qualified elsewhere in INSERT ON
CONFLICT, and I think this patch adds more weight to that argument.
[1] https://www.postgresql.org/message-id/flat/a31a76b6-b176-47fe-8778-9dfece231341@wikimedia.org
So should we do that, and allow unqualified column names in ON
CONFLICT SET and WHERE?
In my opinion, yes. Qualifying with the table name doesn’t really make it much clearer which version of the table you’re referring to than having it unqualified.