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

Lack of support for composite keys #449

Closed
HoneyryderChuck opened this issue Nov 14, 2012 · 2 comments
Closed

Lack of support for composite keys #449

HoneyryderChuck opened this issue Nov 14, 2012 · 2 comments

Comments

@HoneyryderChuck
Copy link

I've just come across an issue concerning active record implementation with composite keys. I'm switching our "Join" models to an implementation without a unique primary key using this gem: https://github.com/drnic/composite_primary_keys. Thing is, when one of its instances calls gets delayed, the instance gets rightfully dumped to the handler, but on load it can't get instantiated anymore. The main offender is this line here:

# psych_ext: line 100
id = payload["attributes"][klass.primary_key]

This works fine for the case where the primary key is one key, but not for when the primary_key returns an array of keys, which would be the case with a composite key implementation. To get around this issue, I had to "hack" the Hash class of ruby itself, so that it would respond well with an array given as key:

class Hash
  def selector_with_array(*args)
    if (f = args.first).is_a?(Array) and self.none?{|k, v| k.is_a?(Array)}
      a = self.values_at(*f).compact
      a.empty? ? nil : a
    else
      selector_without_array(*args)
    end
  end

  alias_method :selector_without_array, :[]
  alias_method :[], :selector_with_array

end

and hacking is ugly is hell. The cleanest solution, in my eyes, would be to rewrite that particular line described above, to:

#psych_ext: line 100
id = payload["attributes"].values_at(*klass.primary_key)

It would work anyway the way delayed job is expected to work, and it wouldn't necessarily be a hack to get around an external gem, just a different hash operation use which would deliver the same result.

You may argue that, since AR doesn't support composite keys natively, why should you bother? Well, I think this is of course an AR flaw, not yours. But DataMapper, to name an AR alternative, supports composite keys. As I've seen, you don't currently provide support for DataMapper. But in case you one day do, this will be probably an issue, I'd say.

@albus522
Copy link
Member

I will take a look at some solutions, but I feel I should point out that you will find very little real support for composite primary keys using Rails, and especially with ActiveRecord. If you don't have an extremely good reason for making this switch, I would highly recommend leaving the original primary key alone.

@HoneyryderChuck
Copy link
Author

Hey, just added a pull request with a solution proposal (I don't know how to reference issues directly, so here's the pull url: #451) .

About the support, as stated, I get from the referenced gem. I actually don't think this is a Delayed Job nuisance, but an AR nuisance overall, and a bit of a side-effect of the whole "convention over configuration" bla. If I have a model which is basically a many-to-many join model, why do I need the id there?

I've seen there is already a datamapper version of delayed jobs. How did you circumvent this issue, since DM supports composite keys out of the box? Does it work for "composited" DM models?

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

No branches or pull requests

2 participants