One()/Next() : Unable to detect empty results

[jfbus]

type Data struct {
  foo int
}

v := Data{}
r.Table("table").Get("doesnotexist").Run(session).One(&v)

nothing tells me if the row exists or not

Wouldn't it be better for One (+ Scan) to return an error in this case ?

err := r.Table("table").Get("doesnotexist").Run(session).One(&v)
if _, ok := err.(ErrNullResult); ok {
  // was not found, do something
}

rows := r.Table("table").Get("doesnotexist").Run(session)
rows.Next()

returns true, even if there is nothing in the DB
one would expect to get false

[christopherhesse]

Looks good, I'll try it out and merge it when I get the chance.

[elithrar]

In the current version of the driver, you can always:

var response User
err := r.Table("users").Get("doesnotexist").Run(session).One(&response)

if response.Id == "" {
  // return User{}, custom err
}

Similarly, for .All responses you can check len(response) == 0 where response is a []User—or whatever other slice of structs/maps you are using to marshal your response into.

[christopherhesse]

@elithrar I'm leaning toward adding this in, because otherwise it seems a bit hard to detect. What do you think?

[jfbus]

Seems awkward to test Id == "" if nothing is found.

Also, if Id is an integer, you would have to test Id == 0, and 0 should be a valid Id value

[elithrar]

@jfbus Note that I'm assuming that response.Id is the auto-generated id from RethinkDB, which is a string. If you're generating your own id's, then treat my snippet as an example only :)

@christopherhesse The Python & Ruby drivers return null—which doesn't really map across to the Go implementation—so I'm not too fussed.

Note that I'm not against having more descriptive errors—I'm just wanting to make sure this is fully fleshed out and/or less verbose than the current option. I also think .All should have a similar implementation if we are to include an err.ErrNilResult (nil? null? zero? no?) error.

[jfbus]

IMHO, .One and .All should not behave exactly the same.

The expected result for .One is nil (for which I don't see a better solution than returning an error), and, for .All, it is [] (the current behavior).

As far as the name is concerned, I started with ErrNoResult and later renamed to ErrNullResult because I found it better suited to queries like r.do(null).

[christopherhesse]

Another option might be to do what the sql module does: http://golang.org/pkg/database/sql/#NullString

This would presumably mean the user would have to make these container types, which doesn't sound like the greatest.

[jfbus]

Doesn't look good indeed.

The error approach (used by the mongodb driver mgo) is much cleaner.

[christopherhesse]

How about "rethinkdb: Null result", not found may be confusing in certain situations.

[christopherhesse]

I gave you access so you can merge it when it's good to go.

[christopherhesse]

So I like this idea, but what about .All()? I think it would be confusing if you're iterating through a stream that may contain a null in it. If you got [item1 null item2] you'd end up with [item].

[christopherhesse]

Also what if you try to scan into a type that does support being nil? Maybe there shouldn't be an error in that case.

[jfbus]

You are right.

Here is a new proposal :

• One() returns ErrNullResult
• All() returns an empty slice
• Scan() and Next() work as before
• we add func (rows *Rows) IsNull() bool to be able to test for nullrows

[christopherhesse]

But should All() return an empty slice if I'm, for instance, scanning into []interface{} or (not sure if this works) []*string?