Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implemented Compound Indexes #235

Merged
merged 9 commits into from
Oct 4, 2015
Merged

Implemented Compound Indexes #235

merged 9 commits into from
Oct 4, 2015

Conversation

mfenniak
Copy link
Owner

@mfenniak mfenniak commented Oct 3, 2015

Updated @nkreipke's PR #229 to include integration tests.

@mfenniak
Copy link
Owner Author

mfenniak commented Oct 4, 2015

@nkreipke, just want to let you know that I made an API change in this PR while writing tests for it. I liked the direction you took it, but I didn't like the disconnect between the index definition and then accessing any data via the index; it was possible to be type-safe on either side, but the types weren't related. So, I changed the API to use the IndexDefine method, like so:

var index = testTable.IndexDefine("Compound", a => a.Name, a => a.SomeNumber);
// index is now an object that knows it is a compound index of type <String, Double>
connection.Run(index.IndexCreate());

// index.Key() method is a type-safe method to create a compound index key object
var key = index.Key("Jim Bob", 100);

// index.GetAll() is an existing index extension method that ensures the key being
// accessed matches the type of the index; in this case it must be a
// CompoundKey<String, Double>; similar extension methods exist
// for Between, and other operations that can use secondary indexes
connection.Run(index.GetAll(key))

mfenniak added a commit that referenced this pull request Oct 4, 2015
@mfenniak mfenniak merged commit 87ffe57 into master Oct 4, 2015
@mfenniak mfenniak deleted the compound_indexes branch October 4, 2015 03:59
@nkreipke
Copy link
Contributor

nkreipke commented Oct 4, 2015

I like this, it looks far better than what I've come up with. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants