Permalink
Show file tree
Hide file tree
10 comments
on commit
sign in to comment.
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Browse files
cast hstore values on write to be consistent with reading from the db.
- Loading branch information
Showing
4 changed files
with
34 additions
and
4 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
5ac2341
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/cc @tenderlove
5ac2341
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
5ac2341
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
git-bisect is telling me that this commit broke store_accessor for hstores.
Test code at https://gist.github.com/inopinatus/6850497
5ac2341
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@inopinatus I can confirm your problem.
The issue is that after the patch writing and reading an
hstore
column always results in aHash
. Even when you tried to set aHashWithIndifferentAccess
. Asstore.rb
needs aHashWithIndifferentAccess
to work properly things go bad.I'll look into a possible solution.
5ac2341
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@senny Any updates on this? The problem raised by @inopinatus is still broken in 4.1 as it is.
5ac2341
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ffortin just checked and the test script above passes on both
master
and4-1-stable
.5ac2341
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@senny My bad, I've published an updated gist, the problem happens when duplicating an object with hstore properties. Might be worth filing under a separate issue?
See: https://gist.github.com/ffortin/04469945cc2e47a0c56c
This used to work under 4.0
5ac2341
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ffortin I am still not able to reproduce. I ran the gist against
master
and4-1-stable
and it passes...5ac2341
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@senny Ok sorry, we were testing with 4.1.5, it was resolved in 4.1.6, by you in fact.
"Keep PostgreSQL hstore and json attributes as Hash in @attributes. Fixes duplication in combination with store_accessor.
Fixes #15369.
Yves Senn"
Thanks and sorry for bothering you!
5ac2341
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ffortin thanks for confirming😁