Skip to content

Conversation

@wehriam
Copy link

@wehriam wehriam commented Feb 16, 2012

I'm not sure of the correct version number for a master merge.

@edevil
Copy link

edevil commented Mar 15, 2012

Is there still a problem with this patch?

@wehriam
Copy link
Author

wehriam commented Mar 19, 2012

@edevil not sure. @thepaul ?

@thepaul
Copy link
Collaborator

thepaul commented Mar 19, 2012

Not one I know of. I need to get a chance to investigate it more thoroughly before committing, though. It's on the radar.

@thobbs
Copy link
Collaborator

thobbs commented Mar 29, 2012

@wehriam I have a few questions and suggestions on the pull request.

First, I think the batch functions for counters are definitely needed and almost good to go. I would lean towards combining batch_add and batch_multikey_add; offer only the multikey functionality, but simply call it batch_add. Thoughts?

I'm a little skeptical about the value added by batch_remove_rows. The batch_remove method already offers something similar but is more powerful. Am I missing an advantage? I have similar thoughts about batch_multikey_insert versus the existing batch_mutate.

@wehriam
Copy link
Author

wehriam commented Apr 2, 2012

I agree batch_multikey_add and batch_add should be consolidated.

It is my understanding that batch_remove requires a user to pass names, start, or finish to create a slice predicate/range, or else no columns are removed. (batch_remove_rows removes entire rows.) If batch_remove can be already be used to remove multiple whole rows, I agree batch_remove_rows should be discarded. Otherwise, batch_remove should be updated to include the removal of whole rows without having to specify column names or ranges.

Looking at the tests it appears batch_multikey_insert duplicates batch_mutate functionality.

@thobbs
Copy link
Collaborator

thobbs commented Apr 2, 2012

I'm not sure why batch_remove even has the start, finish, reverse, or count parameters. Having a SliceRange on a SlicePredicate in a Deletion object isn't allowed. As far as Cassandra is concerned, if names is None, it will delete the entire row.

batch_remove should just create the SlicePredicate itself instead of going through _mkpred() and only set its column_names attribute to whatever is passed in as names. This would give it the correct behavior of deleting the entire row if names is None.

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.

4 participants