dmfay / massive-js

A data mapper for Node.js and PostgreSQL.
2.49k stars 158 forks source link

'IS' / 'IS NOT' with value: true should be interpreted the same as for value: null #625

Closed Sayene closed 6 years ago

Sayene commented 6 years ago

Summary

Now, there is no option to set where / conditions to use 'IS NOT true'. This works for 'IS NOT null' but not for true value.

Code demonstrating the behavior

db.things.find({'field is not':true}); => SQL error
db.things.find({'field is not':null}); => correct SQL : 'where field IS NOT null'
db.things.find({'field <>':true}); => SQL: 'where field <> true' that obviously is not working like 'where field IS NOT true'

Expected behavior

db.things.find({'field is not':true}); should generate : 'where field IS NOT true'

fixing seems to be as simple as changing where.js, line 136 (v.2.7.5) from: "if (value === null) {" to: "if (value === null || value === true) {"

dmfay commented 6 years ago

It took me a bit to realize you were talking about v2 -- this definitely works in trunk! :laughing: Out of curiosity, what's standing in the way of upgrading?

Outside security updates, I'm not prioritizing work on older versions, but if you'd like to put true/false detection and a testcase proving it out into a pull request against the v2 branch I'd be happy to merge and cut a patch release.

Sayene commented 6 years ago

I'm sorry, I should have made it more clear it was v2. And yes, good point - why not upgraded yet - guess it's mostly due to some 'convenience' wrapper we use and simple lack of time/resource ... But having checked great progress done in v5 - I think it's time to catch up! Thanks for the great lib!