neumino / thinky

JavaScript ORM for RethinkDB
http://justonepixel.com/thinky/
Other
1.12k stars 128 forks source link

undefined values are not handled correctly for filter operation #616

Open riteshsangwan opened 7 years ago

riteshsangwan commented 7 years ago

Continuing from here

ReQL doesn't have an undefined datatype. What would be convenient for the users is to implicitly treat undefined as NULL and pass NULL to rethinkdb. This conversion should be handled at thinky layer.

const users = yield User.filter(r.row('username').eq(username).or(r.row('email').eq(email)));

This will throw error

Cannot convert `undefined` with r.expr() in:\nr.table("users").filter(r.row("username").eq(undefined).or(r.row("email").eq(undefined)))

As a workaround below code works

const users = yield User.filter(r.row('username').eq(username || NULL).or(r.row('email').eq(email || NULL)));

Logically since I have a users table where email or username cannot be null/undefined I would expect this query to return 0 results. This is what mongo returns since many of the developers actively work on mongo we would expect this to work the same way.

What I would like is for this query to not throw error and return 0 results.

I have been reading the docs and is not able to find the solution. How to get this done in rethink world.

const users = yield User.filter({ username: undefined });

This will return all the users and completely ignore undefined value for username. This is logically not correct since ideally this should not return any results because there are no records where username is undefined.

Additional Info

User is a thinky model

const User = thinky.createModel('users', {
    id: type.string().uuid(4),
    fullName: type.string().required(),
    username: type.string().required(),
    email: type.string().email().required(),
    password: type.string().required(),
    firstName: type.string().alphanum().allowNull(true),
    lastName: type.string().alphanum().allowNull(true),
    bio: type.string().allowNull(true),
    // the timestamp this resource is created
    createdAt: type.number().integer().required(),
    // the timestamp this resource was last updated
    updatedAt: type.number().integer().required(),
  }, {
    enforce_extra: 'remove',
    enforce_type: 'strict',
  });