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

eqJoin method not found #16

Closed
ptrln opened this issue Nov 10, 2012 · 9 comments
Closed

eqJoin method not found #16

ptrln opened this issue Nov 10, 2012 · 9 comments
Assignees
Milestone

Comments

@ptrln
Copy link

ptrln commented Nov 10, 2012

Hi,

I just read about rethinkdb on HN, and I downloaded it and tried it out. Seems pretty good so far, and I'm trying out the joins. I tried to run eqJoin, and when I tried to run:

r.table('posts').eqJoin('user_id', r.table('users')).runp()

I got this error:

TypeError: Object [object Function] has no method 'eqJoin'

I have no problems running innerJoin like this:

r.table('posts').innerJoin(r.table('users'), function(post, user){return post('user_id').eq(user('user_id'))}).runp()

Thanks!

@coffeemug
Copy link
Contributor

@ptrln could you try equiJoin instead of eqJoin? I believe it's a bug in the JS client (the name has changed, but somehow I guess it wasn't updated). We'll fix it first thing Monday.

@ghost ghost assigned wmrowan Nov 10, 2012
@ptrln
Copy link
Author

ptrln commented Nov 10, 2012

yeap, equiJoin worked. However, when I run it I get this error:

> r.table('posts').equiJoin('user_id', r.table('users')).runp()
undefined
> { name: 'Runtime Error',
  message: 'Attribute: id is not the primary key (user_id) and thus cannot be selected upon.\n\tr.table(\'posts\').concatMap(function(arg0_1) {return r.let({\'right\':r.table(\'users\').get(r.letVar(\'arg0_1\')(\'user_id\'), \'id\')}, r.branch(r.letVar(\'right\').ne(r.expr(null)), r.expr([r.expr({\'left\':r.letVar(\'arg0_1\'), \'right\':r.letVar(\'right\')})]), r.expr([])))})\n\t                                                                                                                       ^^^^                                                                                                                                         ' }

But when I run the innerJoin version, it works fine:

> r.table('posts').innerJoin(r.table('users'), function(post, user) {return post('user_id').eq(user('user_id'))}).runp()
undefined
> { left: 
   { title: 'Google',
     url: 'http://google.com',
     user_id: 'peterlin',
     sub_id: 'news',
     post_id: '17af9cb1-e4e6-46b8-8963-fca62d059916' },
  right: { user_id: 'peterlin', name: 'Peter'} }

Is it because eqJoin can only be run on the primary key? The API docs isn't clear on this. I assumed "An inner join with a simple equality filter on the given attribute." means by two queries are equivalent.

@coffeemug
Copy link
Contributor

Confirmed that's the bug. @wmrowan -- could you change equiJoin to eqJoin in the JS client?

@coffeemug
Copy link
Contributor

@ptrln -- yes, that's correct, eqJoin only works when you're joining on the primary key of the right table (you can join on any attribute in the left table). Thanks for pointing this out, we'll clarify the docs together with this fix.

@mlucy
Copy link
Member

mlucy commented Nov 10, 2012

So, I might be reading this wrong, but I believe the problem is that right now all the clients hard-code 'id' as the primary key in a few places, including as the key we select on for the right table in an eq_join.

At least in the Ruby client, the syntax here is that the first argument is the attribute we're selecting from the left table, and then we always use id as the attribute of the right table.

We should just fix this on the server, so that instead of hardcoding the id in the client that part of the protobuf is optional and it's filled in by the server as the primary key of the table.

A simpler client-only fix would be to reintroduce the optional third argument to eq_join, and let users specify the primary key of the right table there. This would also be forward-compatible when we introduce secondary indexes. I'll open a separate issue about that.

@coffeemug
Copy link
Contributor

@mlucy eq_join still has an optional third argument as best as I can tell (though it seems to be missing from the docs).

@mlucy
Copy link
Member

mlucy commented Nov 10, 2012

Hm, it doesn't in Ruby. I'll fix that and update the docs.

@ghost ghost assigned mlucy Nov 10, 2012
@coffeemug
Copy link
Contributor

For this issue, we need to change the JS client from equiJoin to eqJoin, and add a third argument to the Ruby client. We also need to verify that the docs are correct on everything and have good examples.

For deeper server updates, I opened #29.

@ghost ghost assigned wmrowan Nov 11, 2012
@wmrowan
Copy link
Contributor

wmrowan commented Nov 13, 2012

This has been fixed.

@wmrowan wmrowan closed this as completed Nov 13, 2012
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

4 participants