On Tue, 7 Oct 2025 at 14:56, Viktor Holmberg <[email protected]> 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
