Have client insert functions return insert result instead of raw job row (breaking)

[brandur]

This one's aimed at resolving the problem described in this discussion
[1]. In short, when an inserted job with unique properties in skipped
because it was a duplicate, there's no easy way to ascertain through
River's API that this is what happened.

The cleanest resolution is what's proposed here. Instead of returning a
raw JobRow, insert functions return a JobInsertResult that contains
a job row, along with a boolean indicating whether an insertion was
skipped due to a duplicate unique. The result struct is also better for
future compatibility in that it could contain other metadata pertaining
to job insertions.

The downside of this approach is that it's a breaking change. The hope
is that it's not that bad of a breaking change because most of the
time callers don't care that much about properties on a return result
row given they just sent most of it as a parameter to Insert, so they
might not be accessing any return values and therefore not be broken.

You can get a rough idea for changes in callers by looking at what
changed in this diff. client_test.go needed quite a few tweaks because
it's doing some more comprehensive testing, but only one example test
(more representative of user code) had to change, which is a good sign.

[1] #86

[brandur]

@bgentry So I'm kind of warming up to just making this (hopefully minor) breaking change because it feels like by far the most "correct" design, and I have a theory that most users won't be accessing the insert return values anyway, so it won't be breaking for most users.

I'm thinking this: we ship this breaking change, but in the same version as other breaking changes we know of: #236, and one other small one that I'll tee up soon. Shipping all breaking changes in one release doesn't make them any less breaking, but it means that when users upgrade, they only have to fix things once, and hopefully not for a long time again. Thoughts?

I added a new "breaking change" tag to track these.

[bgentry]

I think it makes sense to rip this bandaid off and I don't think the breakage will be too bad (obvious compiler errors and simple fix).

I do wonder if it'll be an issue that the InsertMany APIs don't have the ability to return this data, but then those aren't checked for uniqueness anyway. However if that API is going to continue relying on COPY FROM, then it really can't support flexible uniqueness. Actually, just realized we don't clearly document the lack of uniqueness support in InsertMany—want to take care of that too?

Finally, this will need a changelog entry.

[brandur]

I do wonder if it'll be an issue that the InsertMany APIs don't have the ability to return this data, but then those aren't checked for uniqueness anyway. However if that API is going to continue relying on COPY FROM, then it really can't support flexible uniqueness.

Yeah, those were my thoughts. It's going to be difficult to support uniqueness ever on InsertMany given how things are today, so it seems better for now just to punt on that. The return values for Insert versus InsertMany are already significantly different today as it is, and even if InsertMany were to respect uniqueness in the future, it might be reasonable to say that it won't return a UniqueSkippedAsDuplicate flag for the same reason InsertMany doesn't currently return JobRows — presumably if you're using it, you care a lot about insert performance, and you'd rather a faster operation than more information return values (which you can easily get by just using normal Insert).

Actually, just realized we don't clearly document the lack of uniqueness support in InsertMany—want to take care of that too?

Added.

Finally, this will need a changelog entry.

Cool. Added.

[brandur]

And as elsewhere, will hold off merge until we're ready for all breaking changes at once.