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

Comment typos, duplicate code removal and avoid unnecessary fetch of column key_ from the database. #188

Merged
merged 3 commits into from Jun 11, 2014

Conversation

d4mian
Copy link
Contributor

@d4mian d4mian commented May 29, 2014

Commit 862affa: comment typos fixes.

Commit d4c46d8: remove duplicate code.

Commit 39dc6cd: key_ column is being fetch from the database while it is set in the WHERE condition (using the value stored in the variable key). Therefore the value stored in the variable key can be used, avoiding the unnecessary fetch of column key_ from the database.

@arunthirupathi
Copy link
Collaborator

Can you give more context on the key_ field change ?

The other 2 commits look fine.

@d4mian
Copy link
Contributor Author

d4mian commented May 31, 2014

Both methods, put and delete, receive the key as a parameter and use it as part of the SQL query where condition (used to fetch the key_ and version_) querying for those records/rows whose key_ column match the given key value.

In the select SQL query the key_ column is requested to match the key value given as parameter to the method, and therefore the fetched rows will have in the key_ column the value of given key (due to where condition).
The key value given as parameter could be used instead of fetching the key_ column from the database.

@arunthirupathi
Copy link
Collaborator

Looked at the code, seems fine to me, merging the request.

arunthirupathi added a commit that referenced this pull request Jun 11, 2014
Comment typos, duplicate code removal and avoid unnecessary fetch of column key_ from the database.
@arunthirupathi arunthirupathi merged commit 99e7207 into voldemort:master Jun 11, 2014
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.

None yet

2 participants