sails-dynamodb icon indicating copy to clipboard operation
sails-dynamodb copied to clipboard

Null and NotNull where conditions not supported

Open jkeczan opened this issue 10 years ago • 4 comments

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?

jkeczan avatar Oct 16 '15 13:10 jkeczan

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

devinivy avatar Oct 16 '15 13:10 devinivy

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.

jkeczan avatar Oct 16 '15 14:10 jkeczan

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

devinivy avatar Oct 16 '15 14:10 devinivy

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.

jkeczan avatar Oct 16 '15 14:10 jkeczan