Re: ON CONFLICT DO SELECT (take 3)

>> I did some simple tests, found out that
>> SELECT FOR UPDATE, the lock mechanism seems to be working as intended.
>> We can add some tests on contrib/pgrowlocks to demonstrate that.
Test added.

>> infer_arbiter_indexes
>>               ereport(ERROR,
>>                       (errcode(ERRCODE_WRONG_OBJECT_TYPE),
>>                        errmsg("ON CONFLICT DO UPDATE not supported
>> with exclusion constraints")));
>> I guess this works for ON CONFLICT SELECT?
>> we can leave some comments on the function infer_arbiter_indexes,
>> and also add some tests on src/test/regress/sql/constraints.sql after line 570.
Good catch. In fact it did “work” as in "not crash" - but I think it shouldn’t.
With exclusion constraints, a single insert can conflict with multiple rows.
I assumed that’s why DO UPDATE doesn’t work with it - because it’d update multiple rows.
For the same reason, I think DO SELECT shouldn’t work either, as you could then
get multiple rows back for a single insert.
I guess in both cases you could make it so it updates/selects all conflicting rows - but I’d
really prefer to leave it as an error. If someone actually wants this to work with exclusion
constraints the error can always be removed in a future version. But if we add a multi-row-return
then we are locked in forever.

>> changing
>> OnConflictSetState
>> to
>> OnConflictActionState
>> could make it a separate patch.
Skipping this for now, let me know if you strongly object.

>> all these 3 patches can be merged together, I think.
Ok done. But these review fixes are separate for ease of review. Before merging they should
be folded in to the main/first commit.

>> "/* DO NOTHING or UPDATE? */"
>> this comment needs to be changed?
Done

>> ----------------------------------------
>> src/backend/rewrite/rewriteHandler.c
>> parsetree->onConflict->action == ONCONFLICT_UPDATE
>> maybe we also need to do some logic to the ONCONFLICT_SELECT
>> (I didn't check this part deeply)
Yes, this needed to be fixed to make updatable views work. Done.
>> If you compare it with the result above, it seems the updatable view behaves
>> inconsistent with ON CONFLICT DO SELECT versus ON CONFLICT DO UPDATE.
Yes, it was wrong. Nice catch. Fixed now I think, and test added.


I believe this new patch addresses all the issues found by Jian. I hope another review won’t find quite so much dirt!