kylefarris / node-querybuilder

Node QueryBuilder Adapter for Node.js (tags: nodejs, node, mysql, active record, activerecord, querybuilder, query builder)
49 stars 19 forks source link

[2.0.0-beta.1] cb is not defined error when insert_batch with empty array #36

Open Flamenco opened 6 years ago

Flamenco commented 6 years ago

driver: mysql

const members = [] // some function that may return an empty array
qb.insert_batch('group_user', members, (err, res) => {})

Expected behavior would be to simply invoke the callback. If the server response with 0 affected rows is needed, then to actually call the DB with no rows to insert.

TypeError: cb is not a function
    at Query._connection.query (/Users/steven/dev/projects/other/quickchords-server/src/main/javascript/node_modules/node-querybuilder/drivers/mysql/query_exec.js:33:17)
    at Query.<anonymous> (/Users/steven/dev/projects/other/quickchords-server/src/main/javascript/node_modules/mysql/lib/Connection.js:502:10)
    at Query._callback (/Users/steven/dev/projects/other/quickchords-server/src/main/javascript/node_modules/mysql/lib/Connection.js:468:16)
    at Query.Sequence.end (/Users/steven/dev/projects/other/quickchords-server/src/main/javascript/node_modules/mysql/lib/protocol/sequences/Sequence.js:83:24)
    at Query.ErrorPacket (/Users/steven/dev/projects/other/quickchords-server/src/main/javascript/node_modules/mysql/lib/protocol/sequences/Query.js:90:8)
    at Protocol._parsePacket (/Users/steven/dev/projects/other/quickchords-server/src/main/javascript/node_modules/mysql/lib/protocol/Protocol.js:278:23)
    at Parser.write (/Users/steven/dev/projects/other/quickchords-server/src/main/javascript/node_modules/mysql/lib/protocol/Parser.js:76:12)
    at Protocol.write (/Users/steven/dev/projects/other/quickchords-server/src/main/javascript/node_modules/mysql/lib/protocol/Protocol.js:38:16)
    at Socket.<anonymous> (/Users/steven/dev/projects/other/quickchords-server/src/main/javascript/node_modules/mysql/lib/Connection.js:91:28)
    at Socket.<anonymous> (/Users/steven/dev/projects/other/quickchords-server/src/main/javascript/node_modules/mysql/lib/Connection.js:502:10)
kylefarris commented 6 years ago

It's interesting, I actually have a test written, running, and passing that should account for this scenario: https://github.com/kylefarris/node-querybuilder/blob/58f1f714a6391cd67da80c515f5399f1152918d4/test/mysql/tests-insert_batch.js#L51

I'll have to do some research...

kylefarris commented 6 years ago

@Flamenco I found the issue--it didn't really have anything to do with the callback, per se... strange/hard bug to track down.

Problem is that when its fixed, it runs the expected a query: INSERT INTO [some_table] () VALUES (). Seems good, but, then if any columns do not have a default value (the case with my mock_db for testing), you'll get errors anyways.

I could simply return the response object with affected_rows: 0 if the dataset is empty but there could conceivably be a reason for seeding a database with a bunch of rows of default data (with, say, only an incrementing primary key).

I'm not really sure there is a great way to get around this issue other than simply not allowing empty arrays in the insert_batch() method. Do you have any suggestions?

Flamenco commented 6 years ago

I suggest to return affected_rows: 0 and also add another property bypassed:true. We can use that property to signal here, and in other instances, that the database was not hit.

Flamenco commented 6 years ago

Also, in the main configuration, there can be an option to allow bypassing the database. (allowBypass:true). That will make it very clear that some requests, such as this, will not hit the wire.