Skip to content

Explictly check return from lock functions#11

Closed
ozzyaaron wants to merge 1 commit into
ClosureTree:masterfrom
agworld:fix-postgres-locking-selects
Closed

Explictly check return from lock functions#11
ozzyaaron wants to merge 1 commit into
ClosureTree:masterfrom
agworld:fix-postgres-locking-selects

Conversation

@ozzyaaron

Copy link
Copy Markdown

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
Copy Markdown

👍

@mceachen

Copy link
Copy Markdown
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
Copy Markdown
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