Avoid unnecessary UPDATE on conflicted column when upserting

[wolfgangwalther]

This is supposed to fix #2446.

I'm not sure whether we should go ahead, though. For one, it's a breaking change, so we shouldn't merge it right away. That's because of this test failing:

  test/spec/Feature/Query/UpsertSpec.hs:96:13:
  1) Feature.Query.UpsertSpec.UPSERT, with POST, when Prefer: resolution=merge-duplicates is specified, succeeds if the table has only PK cols and no other cols
       body mismatch:
         expected: [{"id":1},{"id":2},{"id":4}]
         but got:  [{"id":4}]

results for Prefer: return=representation will be different, because fewer rows are actually updated - and thus returned.

---

Furthermore, the change doesn't seem to be as simple as assumed, because now two other tests fail:

  test/spec/Feature/Query/UpsertSpec.hs:389:13:
  2) Feature.Query.UpsertSpec.UPSERT, with PUT, Updating row, succeeds if the table has only PK cols and no other cols
       status mismatch:
         expected: 200
         but got:  400
       body mismatch:
         expected: [{"id":1}]
         but got:  {"code":"PGRST115","details":null,"hint":null,"message":"Payload values do not match URL in primary key column(s)"}

  To rerun use: --match "/Feature.Query.UpsertSpec/UPSERT/with PUT/Updating row/succeeds if the table has only PK cols and no other cols/"

  test/spec/Feature/RpcPreRequestGucsSpec.hs:65:9:
  3) Feature.RpcPreRequestGucsSpec, GUC headers on all methods via pre-request, succeeds setting the headers on PUT
       status mismatch:
         expected: 204
         but got:  400
       unexpected header: Content-Type
       missing header:
         X-Custom-Header: mykey=myval
       the actual headers were:
         Content-Type: application/json; charset=utf-8
       body mismatch:
         expected: ""
         but got:  "{\"code\":\"PGRST115\",\"details\":null,\"hint\":null,\"message\":\"Payload values do not match URL in primary key column(s)\"}"

Since this is not a regression, but potentially a breaking change, we should push this back after the next patch release for sure - if we decide to do it at all.

[steve-chavez]

Since this is not a regression, but potentially a breaking change, we should push this back after the next patch release for sure - if we decide to do it at all

Agree. I didn't expect the patch to be complicated.

(been wrong on beginner tags many times now, maybe I should remove that tag)

[steve-chavez]

Linking this comment, an alternative solution would be to UPDATE according to the role privileges.