goodybag / mongo-sql

An extensible SQL generation library for JavaScript with a focus on introspectibility
321 stars 72 forks source link

$in query should be dropped if array is empty #184

Closed ravshansbox closed 6 years ago

CvBlixen commented 6 years ago

This is very similar to #124 and I have also had problems with that.

... but this seems to have been fixed in 6.0.0:

https://github.com/goodybag/mongo-sql/commit/8f2bb01daf23ef7de889de5be2a1da250acafaad

ravshansbox commented 6 years ago

I've tested with v6.1.0, it is still generating incorrect output. For query object { type: 'select', table: 'table1', where: { a: { $in: [] } } }, it is generating select "table1".* from "table1" where false, but expected select "table1".* from "table1"

CvBlixen commented 6 years ago

Oh! But I thought that was how it was supposed to be. 🤔

There's even a test that verifies that this is the case:

https://github.com/goodybag/mongo-sql/blob/master/test/conditions.js#L392

I think the rationale behind this is that false is safer than (implied) true in for instance the case of an update query.

I propose the following addition to the docs:

https://github.com/goodybag/mongo-sql/pull/185

jrf0110 commented 6 years ago

[edit] whoops, accidentally hit comment and close before actually submitting my comment.

Looks like this was changed in https://github.com/goodybag/mongo-sql/pull/180

After reading @johansteffner's rationale, I tend to agree with him. If you set the $in operator and supply an empty list, then indeed, no value would be in that list and the condition would resolve to false.

However, I'm open to suggestions here

ravshansbox commented 6 years ago

After thinking a while, I got the idea behind it. I agree with @CvBlixen, it's working the way it was supposed, thanks.