Skip to content

Add an option to make queries always return an array #1367

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

Open
wraithan opened this issue Mar 12, 2016 · 9 comments
Open

Add an option to make queries always return an array #1367

wraithan opened this issue Mar 12, 2016 · 9 comments

Comments

@wraithan
Copy link

Specifically I'm talking about: https://github.com/felixge/node-mysql/blob/master/lib/protocol/sequences/Query.js#L131-L145

We were bitten by a bug where we assumed a value (Object) was coming back (because it did before) then we changed some code and it then started returning an Array. Convention (like always doing type checks) can be inconsistent, and writing a wrapper for this feels messy since we are creating a new array to put the value into after it was just popped out of an array.

Since it would be a backwards incompatible change to turn this on for everyone, I'd like to suggest having an option.

Supposing you agree to this feature, if there are other areas that need to be patched, it would be awesome for you core devs to point them out in this ticket. That way if someone decides to write a patch it will catch the other cases of this, if they exist.

@sidorares
Copy link
Member

related issues: #770 #658

@wraithan
Copy link
Author

I realize I didn't explain myself very well. I'd say that #770 and #658 aren't related. They are about getting an Array instead of an Object of the values.

It is about the number of rows being returned and whether they are being wrapped up in an array or not. If only 1 row is returned, you unwrap it, if multiple rows are returned you return them as an array. I'd like it to alway be wrapped in an array, regardless of the number of results.

@dougwilson
Copy link
Member

Hi @wraithan, perhaps there is still not a very clear explanation going on? Even when we return one data row from a SQL query, it is still an array of one data row object:

db.query('SELECT * FROM information_schema.tables LIMIT 1', function (err, rows) {
  console.log('type: ' + typeof rows);
  console.log('isarray: ' + Array.isArray(rows));
});

Gives me:

type: object
isarray: true

Perhaps if you can give an example of the code that returns a non-array, then the change you did, and the code that is now returning an array?

@sidorares
Copy link
Member

oh, I see. IFAIK the code you are linking returns single array for non-multiple statements result set and array of arrays when there are multiple statements - one array per result

@wraithan
Copy link
Author

Yeah, sorry I also misread the internal bug report, couple hours past the end of the day :). But yes, we do a lot of multi query operations to minimize round trips. We'd like to always get back an array of row sets, that way the return value is always the same. We have at least 1 case of a multi query operation being built up then sent that will have >=1 query in it, making us have special code around handling the results.

@sidorares
Copy link
Member

I'm +1 on this. It's really hard to see what's inside result unless you attach 'fields' event and count number of times it's called before 'end' event

@q42jaap
Copy link

q42jaap commented Apr 1, 2016

-1, I'd suggest a different api specifically for multiple statements. That api would then always return an array. The current api cannot be changed this dramatically, imho.

@wraithan
Copy link
Author

wraithan commented Apr 1, 2016

@q42jaap I was suggesting it as an option, not as a change for everyone. Another API is also an way but would be confusing when this API also support multiple queries.

@millerbryan
Copy link

+1, I would like to see some method of selecting the return object - either an object or an array. I currently have helper methods to run map() against the return and strip off the LHS wheh only an array is needed but is not as elegant as I would like.

dvejmz pushed a commit to infinityworks/node-app-data-connectors that referenced this issue May 9, 2018
- at the moment the return type is different depending on how many
  queries were run.
- See: mysqljs/mysql#1367
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

5 participants