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

pk()/fk()/_get() don't support composite primary keys #63

HNygard opened this Issue Dec 23, 2009 · 1 comment


None yet
2 participants

HNygard commented Dec 23, 2009

I'm having a little problem with the joins on a ManyToMany-relationship since pk()/fk()/_get() is not correctly supporting composite primary keys (multiple primarys).

From _get() I find the following line:

$query = DB::select()
->on($model->fk($field->through), '=', $model->pk(TRUE))
->where($this->fk($field->through), '=', $this->{$this->_primary_key});

$model->pk(TRUE) will in my current case return "rights.Array" since the primary key is an array for two keys from two other tables. Exception tells me:

Database_Exception [ 1054 ]: Unknown column 'rights_users.right_Array' in 'on clause' [ SELECT rights.account_id, rights.rights_template_id FROM rights JOIN rights_users ON (rights_users.right_Array = rights.Array) WHERE rights_users.user_id = 106 ]

I think a solution is to rewrite some of _get() to make support for the different types of primary keys ($this->_primary_key is string/array) and to make a pk()/fk() method that supports the same. An example for the query above is to:

$query = DB::select()->join($field->through);
foreach($_primary_key as $key) {
$query->on($model->fk($field->through, $key), '=', $model->pk(TRUE, $key))
$query->where($this->fk($field->through), '=', $this->{$this->_primary_key});

Is this a usable solution?

banks commented Dec 23, 2009

Shadowhand has intentionally not supported composite primary keys when using relationships. I believe someone forked a while back to add this support but I have not seen that maintained for a while. If you need this, you could fork and add it. I though that any model complex enough to need composite primary keys is probably an edge case and will need special behavior with relationship handling making it more sensible to manage composite PK model relationships manually anyway.

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