-
Notifications
You must be signed in to change notification settings - Fork 39
Added CompositeCodec to support reading/writing composite columns #4
base: master
Are you sure you want to change the base?
Conversation
import java.nio.ByteBuffer | ||
|
||
object CompositeCodec { | ||
def apply[A](codec: Codec[A]) : CompositeCodec[A] = new CompositeCodec(codec) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our style guide says no spaces between the')' and the ':'.
In addition to the above detail-oriented comments, I have a couple of high-level comments–
|
No problem on the detail changes, and yes private[this] is technically what I meant, and I'm happy to make that change. In practice I find it makes little real difference, and it makes the code look more cluttered. But I get your rationale. Good catch on the var/mutable. Regarding heterogeneous types, this is supported if you use the ByteArrayCodec, in which case you're responsible for whatever encoding you want to do prior to constructing the Composite. We follow this pattern in some cases. I originally started down the path of passing in a list of codecs, then handling the encoding inside CompositeCodec, but I didn't do this because it really complicates the typing. Look at Hector's implementation if you want to see what I mean. I do agree that it would make the client code cleaner. |
This latest revision moves the encoding scheme into the component rather than being at the codec level. The issue with composites is the complex typing involved. Specifically, you may write a composite of type Long:UUID:UTF8, then query only the Long portion (but using multiple components of type Long to generate your range predicate), then read the entire thing. The current codec mechanism doesn't provide this much flexibility, and type erasure prevents us from doing much in the way of runtime magic. So after much wrangling, this is my solution:
I look forward to your feedback... |
I should note that I have not successfully built Cassie, and this has been tested as a bolt-on component in our own codebase. It should compile correctly, but I can't test to make sure. |
I have create a standalone CompositeCodec here: https://github.com/TheWeatherChannel/cassie-composite. |
Robbie Strickland seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
This codec allows for operations on composite columns, which is currently not supported in Cassie. Example usage: