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.
/Viktor
Show quoted text
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 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.
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 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.
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
Attachments:
v5-0001-Allow-EXCLUDED-in-RETURNING-list-of-INSERT-ON-CON.patchtext/x-patch; charset=US-ASCII; name=v5-0001-Allow-EXCLUDED-in-RETURNING-list-of-INSERT-ON-CON.patchDownload+747-257
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]/messages/by-id/a31a76b6-b176-47fe-8778-9dfece231341@wikimedia.org 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]: /messages/by-id/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] /messages/by-id/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.