Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Multiple assignments to same column due to duplicate keys in @changed_attributes #110

Merged
merged 3 commits into from Jun 8, 2012

Conversation

Projects
None yet
2 participants
Contributor

jnv commented Jun 4, 2012

I've encountered quite rare problem with CPK while using Devise or CarrierWave. Under some conditions, @changed_attributes hash (used by AttributeMethods::Dirty) can contain duplicated attributes – symbol and string. See ce7eb75 for straightforward example. In the end, the resulting query contains 2 assignments to the same field:

UPDATE "reference_types" SET "abbreviation" = 'b', "abbreviation" = 'b' WHERE "reference_types"."reference_type_id" = 1

The funny part is that most DB systems happily accept such query. PostgreSQL does not.

Rails already prevent this by conversion of attribute key to string (ee6aa8f).
I also had to fix some associations test since the hash from ActiveModel::Dirty#changes uses string keys for assertion (56df46c) – otherwise this shouldn't affect anyone since this method returns a HashWithIndifferentAccess.

Anyway, I think that the preferred solution would be to call the original method, since both are basically same (except for intercepting an Array).
I'm not sure how to do it since I don't understand how the method is overridden in this case – alias :[]= :write_attribute doesn't ring any bells.

@cfis cfis added a commit that referenced this pull request Jun 8, 2012

@cfis cfis Merge pull request #110 from jnv/write_attribute
Multiple assignments to same column due to duplicate keys in @changed_attributes
d558e89

@cfis cfis merged commit d558e89 into composite-primary-keys:master Jun 8, 2012

Contributor

cfis commented Jun 8, 2012

Thanks for the patch.

alias :[]= :write_attribute is something ActiveRecord already does. We have to reapply it though since we override the write_attribute method.

It maps foo[:bar] = 1 to foo.write_attribute(:bar, 1)

Charlie

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment