christopherhesse/rethinkgo

Skip, Limit not worked when used with OuterJoin or InnerJoin

Closed this issue · 7 comments

I used Go 1.1, RethinkDB 1.4.5 and Rethinkgo lastest version.

Data tables

r.db("test").table("A").insert({ Id:1, A1:"A1"});
r.db("test").table("A").insert({ Id:2, A1:"A2"});
r.db("test").table("A").insert({ Id:3, A1:"A3"});
r.db("test").table("B").insert({Id:1,AId:1, B1:1, B2:1});
r.db("test").table("B").insert({Id:2,AId:1, B1:2, B2:1});

Code

package main
import (
    "fmt"
    r "github.com/christopherhesse/rethinkgo"
)
func main() {
    session, err := r.Connect("localhost:28015", "test")
    if err != nil {
    }
    defer session.Close()
    joinF := func(left, right r.Exp) r.Exp {
        return (left.Attr("Id").Eq(right.Attr("AId")))
    }
    var response []interface{}
    r.Table("A").OuterJoin(r.Table("B"), joinF).Skip(0).Limit(10).Run(session).All(&response)
    fmt.Println(response)
}

Print result when NOT using Skip, Limit

[map[left:map[Id:3 A1:A3]] map[right:map[Id:1 B2:1 B1:1 AId:1] left:map[Id:1 A1:A1]] map[right:map[Id:2 B2:1 B1:2 AId:1] left:map[Id:1 A1:A1]] map[left:map[Id:2 A1:A2]]]

Print result when using Skip, Limit

[]

Thanks.

Thanks for the bug report! I will look at it shortly

So it looks like the weird thing here is that with skip or limit you get a single response (SUCCESS_ATOM) instead of a list (SUCCESS_SEQUENCE) from the server. The query you are running is actually returning an error value about this, but you are disregarding it.

The fix in your case is to use .One() instead of .All(). I will see if the docs can make this more obvious.

It doesn't make much sense to me that skip and limit would return SUCCESS_ATOM, so I've made .All() work in your case. You probably should check the returned error value in the future first though, just so you have an idea of what is wrong.

Here's the commit:
cc1000d

Thanks again for the detailed bug report! Good reproduction info, version numbers, and everything!

Thanks a lot

The root cause of this issue is apparently a bug in rethinkdb: rethinkdb/rethinkdb#678 but I will leave the .All() changes in because they seem to make sense.