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

Explictly check return from lock functions #11

Closed

Conversation

ozzyaaron
Copy link

Currently the postgres adapter uses connection#select_value which looking at the documentation the call chain is :

select_value -> select_one -> select_all

select_all "... returns an array of hashes ..."
select_one "... selects first hash of array ..."
select_value " ... selects first value of hash ..."

This means that when selecting more than one value in a statement you need to explictly request which column you want.

In some Rubies maybe select_value always returns the output of the lock functions but at least in REE/MRI 1.8 and jRuby in 1.8 mode where I tested this is not true.

The fix is simple - use execute and retrieve the value ( PR coming ) or change the SQL to only select the lock function result.

Currently the postgres adapter uses connection#select_value which looking at the documentation the call chain is :

select_value -> select_one -> select_all

select_all "... returns an array of hashes ..."
select_one "... selects first hash of array ..."
select_value " ... selects first value of hash ..."

This means that when selecting more than one value in a statement you need to explictly request which column you want.

In some Rubies maybe select_value always returns the output of the lock functions but at least in REE/MRI 1.8 and jRuby in 1.8 mode where I tested this is not true.

The fix is simple - use execute and retrieve the value ( PR coming ) or change the SQL to only select the lock function result.
@joshkinabrew
Copy link

👍

@mceachen
Copy link
Collaborator

Crap, I totally missed this PR. Thanks for the great explanation and the diff—it'll be in v1.1.0 that I will release tonight.

@mceachen mceachen closed this Jun 23, 2014
@ozzyaaron
Copy link
Author

Cool, thanks. I totally missed following up on this too :P

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.

3 participants