gadelkareem / sails-dynamodb

Amazon DynamoDB adapter for Waterline / Sails.js
http://sailsjs.org
46 stars 22 forks source link

Null and NotNull where conditions not supported #17

Open jkeczan opened 8 years ago

jkeczan commented 8 years ago

When testing query using where clause with either 'null' or 'notNull' conditions specified, an error occurs.

Code:

Userreset.findOne({
                        where: {
                            "userID": '12345',
                            "dateUsed": {
                                "notNull": true
                            }
                        }
                    })

Error:

[TypeError: undefined is not a function]
Error (E_UNKNOWN) :: Encountered an unexpected error
TypeError: undefined is not a function
    at Object.module.exports.adapter._applyQueryFilter (/project/node_modules/sails-dynamodb/index.js:691:47)
    at Object.module.exports.adapter.find (/project/node_modules/sails-dynamodb/index.js:594:41)
    at module.exports.find (/usr/local/lib/node_modules/sails/node_modules/waterline/lib/waterline/adapter/dql.js:120:13)

Are these intentionally no longer supported? If so, can we update the documentation, as it can be misleading?

devinivy commented 8 years ago

It should be supported– this is a bug. Will take a PR.

jkeczan commented 8 years ago

After further research, the not null and null are supported; however, they are only supported for scans, not for index queries. With that in mind, I believe that the indexes are being improperly chosen. For example, if you pass through the primary hash and a non-index based column, it will STILL try to use the primary index which results in error.

The solution is to confirm that all columns passed through the where clause are actually apart of an index. If they are not, then a scan should be performed instead.

devinivy commented 8 years ago

Ah, you're probably right. I'm pretty sure that should be detected right around here: https://github.com/gadelkareem/sails-dynamodb/blob/9ca0c93195c012103fb4dd4941e1e7ad2398db2e/index.js#L708

jkeczan commented 8 years ago

We agree as well. We don't have the time to push a PR right now so we wanted to get all of the info here for anybody else.