1602 / jugglingdb

Multi-database ORM for nodejs: redis, mongodb, mysql, sqlite3, postgresql, arango, in-memory...
http://1602.github.io/jugglingdb/
2.05k stars 241 forks source link

where parameters should be normalized on AbstractClass #379

Closed pocesar closed 6 years ago

pocesar commented 10 years ago

The mysql adapter got a "mongodb" way of querying

https://github.com/jugglingdb/mysql-adapter/blob/master/lib/mysql.js#L365

I think this type of conversion should be handled by AbstractClass and the adapter should decide what to do with it. For example, doing a:

User.all({
   where:{
      or:[ 
         {fieldname:{gt:5}},
         {otherfield:null}
      ],
      and:[
        {thirdfield:{lte:4}}
      ],
      fourthfield: {nin:[1,2,3]}
   }
})

should compile to an object that is passed to the adapter, and would be like (arrays can keep the order the conditions are made):

[
  {"or": [
     {"fieldname":[
        {"gt":5}
     ]},
     {"otherfield": [
        null
     ]}
   ]},
  {"and": {
    "thirdfield": [
        {"lte":4}
     ],
    "fourfield": [
        {"nin":[1,2,3]}
    ]
  }}
]

since the AbstractClass already deals with all the queries using _forDB before sending to the adapter, would be a good thing to normalize the query parameters so the implementation on the adapter would be to deal only with an object, for example MYSQL, needs to concat strings, but mongodb doesn't.

For Memory adapter makes sense passing a "predicate" to where, but in a real world scenario you can't query your entire database, then filter out an array.

there's a backbone module that almost does this: https://github.com/bevry/query-engine/blob/master/src/documents/lib/query-engine.js.coffee#L1206

Maybe there's no need to reinvent the wheel :)

1602 commented 10 years ago

I think it makes sense. But I don't want to have coffee-script as a dependency, and, honestly this is too simple functionality to add any dependency at all (I care because jugglingdb core used on client-side and any additional dependency adds more weight to package).

yanickrochon commented 10 years ago

I agree with not having coffee-script as a requirement for jugglingdb core. And it is in my opinion that it should not be a requirement either for the adapters. I believe that Coffee-script is a turn-away for a non negligible user base and should be optional for all official plugins.

pocesar commented 10 years ago

not saying that coffee script should be used, I just pointed out that the code already exists (although the generated coffee script code is clumpsy).

pocesar commented 10 years ago

the thing would be that the major version would need to be changed, and the adapters should follow the new query object