Fix warning when using mysql or sqlite with knex >= 0.16.4

[eldridge]

Fixes #<issue_number_goes_here> (it's a good idea to open an issue first for discussion)

• [x] - npm test succeeds
• [ ] - Code coverage does not decrease (if any source code was changed)
• [ ] - Appropriate JSDoc comments were updated in source code (if applicable)
• [ ] - Approprate changes to js-data.io docs have been suggested ("Suggest Edits" button)

[crobinson42]

@eldridge is this PR complete and are tests working as expected with the changes?

[eldridge]

@crobinson42 yes, the tests passed for me as is on SQLite and MySQL.

I also successfully ran a modified version of the test suite against PostgreSQL. The existing test suite doesn't pass with the pg driver due to column syntax exceptions and differences in identifier quoting, but those are preexisting issues. A diff illustrating the test suite modifications necessary to get the test suite to pass against both master as well as this branch using the pg driver follows:

diff --git a/mocha.start.js b/mocha.start.js
index d41cb84..3c17eb6 100644
--- a/mocha.start.js
+++ b/mocha.start.js
@@ -51,7 +51,22 @@ JSDataAdapterTests.init({
   xmethods: [
     // The adapter extends aren't flexible enough yet, the SQL adapter has
     // required parameters, which aren't passed in the extend tests.
-    'extend'
+    'afterCreate',
+    'afterUpdate',
+    'beforeCreate',
+    'beforeUpdate',
+    'count',
+    'createMany',
+    'create',
+    'destroyAll',
+    'destroy',
+    'extend',
+    'findAll',
+    'find',
+    'sum',
+    'updateAll',
+    'updateMany',
+    'update'
   ],
   xfeatures: [
     'findHasManyLocalKeys',
diff --git a/test/filterQuery.spec.js b/test/filterQuery.spec.js
index 979843c..6ccd628 100644
--- a/test/filterQuery.spec.js
+++ b/test/filterQuery.spec.js
@@ -79,27 +79,27 @@ describe('DSSqlAdapter#filterQuery', function () {
   it('should apply query from array', function () {
     var query = {}
     var sql = adapter.filterQuery(adapter.knex('user'), query).toString()
-    assert.deepEqual(sql, 'select * from `user`')
+    assert.deepEqual(sql, 'select * from "user"')
 
     query = {
       age: 30
     }
     sql = adapter.filterQuery(adapter.knex('user'), query).toString()
-    assert.deepEqual(sql, 'select * from `user` where `age` = 30')
+    assert.deepEqual(sql, 'select * from "user" where "age" = 30')
 
     query = {
       age: 30,
       role: 'admin'
     }
     sql = adapter.filterQuery(adapter.knex('user'), query).toString()
-    assert.deepEqual(sql, 'select * from `user` where `age` = 30 and `role` = \'admin\'')
+    assert.deepEqual(sql, 'select * from "user" where "age" = 30 and "role" = \'admin\'')
 
     query = {
       role: 'admin',
       age: 30
     }
     sql = adapter.filterQuery(adapter.knex('user'), query).toString()
-    assert.deepEqual(sql, 'select * from `user` where `role` = \'admin\' and `age` = 30')
+    assert.deepEqual(sql, 'select * from "user" where "role" = \'admin\' and "age" = 30')
 
     query = {
       role: 'admin',
@@ -112,7 +112,7 @@ describe('DSSqlAdapter#filterQuery', function () {
       ]
     }
     sql = adapter.filterQuery(adapter.knex('user'), query).toString()
-    assert.deepEqual(sql, 'select * from `user` where `role` = \'admin\' and `age` = 30 order by `role` desc, `age` asc limit 5 offset 10')
+    assert.deepEqual(sql, 'select * from "user" where "role" = \'admin\' and "age" = 30 order by "role" desc, "age" asc limit 5 offset 10')
 
     query = {
       where: {
@@ -125,7 +125,7 @@ describe('DSSqlAdapter#filterQuery', function () {
       }
     }
     sql = adapter.filterQuery(adapter.knex('user'), query).toString()
-    assert.deepEqual(sql, 'select * from `user` where `role` = \'admin\' and `age` = 30')
+    assert.deepEqual(sql, 'select * from "user" where "role" = \'admin\' and "age" = 30')
 
     query = {
       where: [
@@ -142,7 +142,7 @@ describe('DSSqlAdapter#filterQuery', function () {
       ]
     }
     sql = adapter.filterQuery(adapter.knex('user'), query).toString()
-    assert.deepEqual(sql, 'select * from `user` where (`role` = \'admin\') and (`age` = 30)')
+    assert.deepEqual(sql, 'select * from "user" where ("role" = \'admin\') and ("age" = 30)')
 
     query = {
       where: [
@@ -174,7 +174,7 @@ describe('DSSqlAdapter#filterQuery', function () {
       ]
     }
     sql = adapter.filterQuery(adapter.knex('user'), query).toString()
-    assert.deepEqual(sql, 'select * from `user` where ((`role` = \'admin\' and `age` = 30) or (`name` = \'John\')) or (`role` = \'dev\' and `age` = 22)')
+    assert.deepEqual(sql, 'select * from "user" where (("role" = \'admin\' and "age" = 30) or ("name" = \'John\')) or ("role" = \'dev\' and "age" = 22)')
   })
   describe('Custom/override query operators', function () {
     it('should use custom query operator if provided', function () {

The changes in this PR result in additional code path, so coverage has dropped a tiny bit, but since testing the change is dependent upon choice of database driver, I don't think there's a good way around it.

[crobinson42]

@eldridge do you think it's worth bumping dependencies that are behind as well in a new release with this?

[crobinson42]

@eldridge do you think it's worth bumping dependencies that are behind as well in a new release with this?

[eldridge]

@crobinson42 I don't have strong opinions there. It's nice to keep dependencies up to date, but I think I want to look at what's changed in knex and any other dependencies since then. I'm using knex 0.19.4 on a new project, but I've been focusing on some other external integrations and have yet to really put the data model through its paces. So far, this has been the only thing that I've run into.

[crobinson42]

Ok, how about I bump dependencies and publish an rc that we can use in our development or staging environments for awhile to see if there are any issues?

On Jan 23, 2020, at 11:28 AM, Mike Eldridge [email protected] wrote:

@crobinson42 https://github.com/crobinson42 I don't have strong opinions there. It's nice to keep dependencies up to date, but I think I want to look at what's changed in knex and any other dependencies since then. I'm using knex 0.19.4 on a new project, but I've been focusing on some other external integrations and have yet to really put the data model through its paces. So far, this has been the only thing that I've run into.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/js-data/js-data-sql/pull/78?email_source=notifications&email_token=ABNSMS42V5WUEWQGF4TTGD3Q7HVUBA5CNFSM4KG4KQKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJYROEI#issuecomment-577836817, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABNSMS6HFS2SLU2XVNYKYGLQ7HVUBANCNFSM4KG4KQKA.