Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Flamenco opened this issue Aug 3, 2018 · 4 comments
Labels

Comments

@Flamenco
Copy link

Flamenco commented Aug 3, 2018

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)
@Flamenco Flamenco changed the title [2.0.beta.1] cb is not defined error when insert_batch with empty array [2.0.0.beta.1] cb is not defined error when insert_batch with empty array Aug 3, 2018
@Flamenco Flamenco changed the title [2.0.0.beta.1] cb is not defined error when insert_batch with empty array [2.0.0-beta.1] cb is not defined error when insert_batch with empty array Aug 3, 2018
@kylefarris
Copy link
Owner

It's interesting, I actually have a test written, running, and passing that should account for this scenario:

expect(() => qb.insert_batch('galaxies',[]), 'empty array provided').to.not.throw(Error);

I'll have to do some research...

@kylefarris
Copy link
Owner

@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
Copy link
Author

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants