Add support for composite column types #8

Merged
merged 14 commits into from Jan 3, 2012

Conversation

Projects
None yet
2 participants
@mstump
Contributor

mstump commented Dec 19, 2011

This is in reference to gh-7, adding composite column support to clj-hector.

It's not quite ready to be merged yet, but I wanted to show you the direction I was headed so you could give feedback early on. It's been several years since I've last written Clojure code so any criticisms would be welcome.

The major issue outstanding is that the Component values that are returned as part of a Composite column when performing a query are always HeapByteBuffers instead of the expected type. After this is fixed I think I might be done.

mstump added some commits Dec 19, 2011

I have composite columns mostly done, the only problem is that compos…
…ite column values in query results are being returned as HeapByteBuffer instead of string
in the toClojure protocol switched from using the concrete type Compo…
…site to AbstractComposite so that we can also deserialze DynamicComposite which shares the same base
finished implementing DynamicComposite and it doesn't look like it's …
…affected by the bug that's holding up Composite
@pingles

This comment has been minimized.

Show comment
Hide comment
@pingles

pingles Dec 21, 2011

Owner

Thanks for all this Matt- looks very interesting.

Apologies I've not taken a look through so far- I've been pretty busy with work but I'm hoping to get a day or two over the next week or so to catch up.

From checking your repository it looks like you've progressed further than these commits?

Owner

pingles commented Dec 21, 2011

Thanks for all this Matt- looks very interesting.

Apologies I've not taken a look through so far- I've been pretty busy with work but I'm hoping to get a day or two over the next week or so to catch up.

From checking your repository it looks like you've progressed further than these commits?

@mstump

This comment has been minimized.

Show comment
Hide comment
@mstump

mstump Dec 21, 2011

Contributor

No problems regarding the delay. I'm not in a huge to push out a new release of clj-hector. I'm pulling in my branch as a leiningen sub project. I just want to be as transparent as possible.

I've completed support for DynamicComposite, and tests for both row fetch, column slice, and column range queries pass tests.

The Composite issue I was running into was most likely a bug in Hector, as other people are seeing it too. I might be able to work around it.

I'm working on keyspace and column family creation with Composite columns, I'm not sure if the issue I'm running into is my bug or Hector's.

I'm going to spend some time working on it today, but starting tomorrow I need to run off and spend time with the relatives so I'm not sure how much progress I'll make.

Adding this feature is taking a little bit longer than I had hoped. I'm paying two penalties, one for coming up to speed on Clojure, and another because Hector isn't particularly well documented.

Contributor

mstump commented Dec 21, 2011

No problems regarding the delay. I'm not in a huge to push out a new release of clj-hector. I'm pulling in my branch as a leiningen sub project. I just want to be as transparent as possible.

I've completed support for DynamicComposite, and tests for both row fetch, column slice, and column range queries pass tests.

The Composite issue I was running into was most likely a bug in Hector, as other people are seeing it too. I might be able to work around it.

I'm working on keyspace and column family creation with Composite columns, I'm not sure if the issue I'm running into is my bug or Hector's.

I'm going to spend some time working on it today, but starting tomorrow I need to run off and spend time with the relatives so I'm not sure how much progress I'll make.

Adding this feature is taking a little bit longer than I had hoped. I'm paying two penalties, one for coming up to speed on Clojure, and another because Hector isn't particularly well documented.

@pingles

This comment has been minimized.

Show comment
Hide comment
@pingles

pingles Dec 22, 2011

Owner

Hi Matt,

I just merged in a bunch of changes + refactorings that Nick had done- it changes some of the assumptions around using maps for super-columns and sounds like it may make some of the composite stuff easier (#12).

I'll see if I can get an hour or two tomorrow to play around with the changes you've been working on. Thanks again for your work!

Owner

pingles commented Dec 22, 2011

Hi Matt,

I just merged in a bunch of changes + refactorings that Nick had done- it changes some of the assumptions around using maps for super-columns and sounds like it may make some of the composite stuff easier (#12).

I'll see if I can get an hour or two tomorrow to play around with the changes you've been working on. Thanks again for your work!

mstump added some commits Dec 27, 2011

discovered bug in CFDef creation where even though we took comparator…
… as an input parameter we weren't setting it on the CFDef
added support for specifying deserializers for Composites, but to do …
…this I needed to add an options parameter to the to-clojure protocol which involved touching lots of code
@mstump

This comment has been minimized.

Show comment
Hide comment
@mstump

mstump Dec 29, 2011

Contributor

Alright, so after a bunch of back and froth about how Composite deserialization is supposed to work on hector-users we finally came to the conclusion that it doesn't which explains the behavior I was seeing.

http://groups.google.com/group/hector-users/browse_thread/thread/ba03b9435689dbbc

As a result I added an option called :c-serializer which allows the user to specify an array of serializers to use for deserializing the Component values of a Composite.

All tests are passing and this pull request is ready for inclusion.

Contributor

mstump commented Dec 29, 2011

Alright, so after a bunch of back and froth about how Composite deserialization is supposed to work on hector-users we finally came to the conclusion that it doesn't which explains the behavior I was seeing.

http://groups.google.com/group/hector-users/browse_thread/thread/ba03b9435689dbbc

As a result I added an option called :c-serializer which allows the user to specify an array of serializers to use for deserializing the Component values of a Composite.

All tests are passing and this pull request is ready for inclusion.

pingles added a commit that referenced this pull request Jan 3, 2012

Merge pull request #8 from mstump/master
Add support for composite column types

@pingles pingles merged commit c0951a4 into pingles:master Jan 3, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment