Projection implementation for MySQL support #177
Conversation
There was a problem hiding this comment.
Since we are breaking backwards compatibility here, can we convert from case class to class too? Similar to how R2dbcSettings is implemented.
There was a problem hiding this comment.
Of course this class is different from R2dbcSettings in a way that users can manually construct it - it is not constructed by config only, so will have to keep the means to set/modify its attributes.
There was a problem hiding this comment.
@pjfanning Do you have a preferred solution for this?
There was a problem hiding this comment.
I see the TODO on the class - so I don't mind a rewrite for 1.1.0.
It would still be nice to minimise source incompatible changes - i.e. code with against 1.0.0 would ideally still be compilable with 1.1.0.
|
I committed a fix for the merge conflict |
| MurmurHash3.finalizeHash(h, values.size) | ||
| } | ||
|
|
||
| private[this] def copy( |
There was a problem hiding this comment.
Technically making copy private is not a source compatible change, but IIUC it is necessary for an implementation that enables keeping backwards compatibility. @pjfanning please let me know if you want this handled differently.
| clock: Clock = Clock.systemUTC()) | ||
| extends R2dbcOffsetStore(projectionId, sourceProvider, system, settings, r2dbcExecutor, clock) { | ||
|
|
||
| override lazy val timestampSql: String = "NOW(6)" |
There was a problem hiding this comment.
This seems like a dialect method, can we abstract a class 'Dialect'? like Hibernate SQL Dialects does.
There was a problem hiding this comment.
We already have an ADT for Dialect, I guess more FP-ish way would be to add a private extension class with these methods, although these kinds of attributes are already present in core.
|
Thanks. I merged this. We can do some tweaks in new PRs if anyone has any issues with these changes. |
Related to #158.