onConflictUpdateSetWhere has wrong type?

[thomasjm]

I noticed that this function's type is

onConflictUpdateSetWhere :: Beamable table
  => (forall s. table (QField s) -> table (QExpr be s) -> QAssignment be s)
  -> (forall s. table (QField s) -> table (QExpr be s) -> QExpr be s Bool)
  -> SqlConflictAction be table

The first callback controls the assignments for the ON CONFLICT UPDATE clause, while the second callback controls the WHERE. It has two arguments,

• table (QField s) -- this represents the existing table
• table (QExpr be s) -- this represents the "excluded" row, i.e. the one that would have been inserted if there were no conflict.

I'm confused about the first argument. Shouldn't it be a table (QExpr be s) instead of table (QField s)? If the purpose is to use it to write a QExpr be s Bool, then it seems wrong for it to have this QField type (unless there's a conversion function I'm missing).

I'm also comparing it with functions like conflictingFieldsWhere, which also use the table (QExpr be s) type.

Please let me know if I'm missing something obvious :)

[thomasjm]

I tried fixing it and the new version worked for me, just submitted a PR :)

[kmicklas]

The QField lets you update the field with <-.. This only applies to the assignment, so we could in theory use QExpr for the condition parameter. I believe both are QField only for consistency, but maybe this was just confusing because it's then inconsistent with the other functions which don't have assignment.

The conversion function you're looking for is current_.

I'll leave this open to track adding better documentation for this.

[kmicklas]

Ah my bad, I see from your PR that you already understood how to use the assignment. Although I think you're right that it would be better to have QExpr for the condition, I'm not sure that it's worth breaking backwards compatibility over it. We should however document the use of current_.