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

Backport 7ba3a48 to 5.0 #26344

Merged
merged 1 commit into from
Aug 31, 2016
Merged

Conversation

sgrif
Copy link
Contributor

@sgrif sgrif commented Aug 31, 2016

No description provided.

I still think that this is something that should be handled in the pg
gem, but it's not going to end up happening there so we'll do it here
instead. Once we bump to pg 0.19 we can pass the encoding to the
`encode` method instead.

This issue occurs because C has no concept of encoding (or strings,
really). The bytes that we pass here when sending the value to the
database will always be interpreted as whatever encoding the connection
is currently configured to use. That means that roundtripping to the
database will lose no information

However, after assigning we round trip through our type system without
hitting the database. The only way that we can do the "correct" thin
here would be to actually give a reference to the connection to the
array type and have it check the current value of the connection's
encoding -- which I'm strongly opposed to. We could also pass in the
encoding when it's constructed, but since that can change independently
of the type I'm not a huge fan of that either.

This feels like a reasonable middle ground, where if we have an array of
strings we simply use the encoding of the string we're given.

Fixes rails#26326.
@rails-bot
Copy link

warning Warning warning

  • Pull requests are usually filed against the master branch for this repo, but this one is against 5-0-stable. Please double check that you specified the right target!

@sgrif sgrif merged commit 2bb2512 into rails:5-0-stable Aug 31, 2016
@sgrif sgrif deleted the sg-backport-7ba3a48-5-0 branch August 31, 2016 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants