Added an option to return resultsets as an array type. #1637
Conversation
|
@ifsnow any progress on the merger? |
|
@Stavanger75 I'm afraid I don't know much about the merging plan. |
| @@ -13,6 +13,22 @@ Object.defineProperty(RowDataPacket.prototype, 'parse', { | |||
| value : parse | |||
| }); | |||
|
|
|||
| Object.defineProperty(RowDataPacket.prototype, 'isArray', { | |||
There was a problem hiding this comment.
Can these be at least underscore-prefixed? Whatever we add here will shadow any column someone may be trying to query, so need to be careful adding anything new.
| @@ -410,6 +411,14 @@ Parser.prototype.packetLength = function packetLength() { | |||
| return this._packetHeader.length + this._longPacketBuffers.size; | |||
| }; | |||
|
|
|||
| Parser.prototype.setArrayRowMode = function(rowsAsArray) { | |||
| this._isArrayRowMode = typeof rowsAsArray === 'undefined' ? this._globalRowsAsArray : rowsAsArray; | |||
There was a problem hiding this comment.
Can you hoist this to prevent the hidden class change this is triggering?
| assert.ifError(err); | ||
| assert.deepEqual(rows, [{1: 1}]); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Can you add a test that verifies the connection is reset back to the mode from the connection config here?
|
@dougwilson added some work to apply your feedback. Please let me know if you need more. |
|
@ifsnow and @dougwilson. Thanks for working on this one :-) |
|
@dougwilson have you checked @ifsnow last changes ? @ifsnow it looks like "eslint" test is failing on missing spaces in the file "test-query-rows-as-array.js"
|
|
@Stavanger75 Yes, I'm done with this. eslint errors were fixed. Thank you for letting me know. |
|
Great !!! |
|
Hi @dougwilson, have you had the time to look at the changes @ifsnow made ? It would be so great if this could be merged into the project. Thanks |
dougwilson
force-pushed
the
master
branch
2 times, most recently
from
638d79d
to
88bade0
Compare
|
Are there plans to merge this in anytime soon? |
|
Any chance this can be merged sometime in the near future? |
|
What about a fork? 🍴 Is someone encouraged? |
|
Also interested by this feature ! What else need to be done to see this merge ? |
|
So taking a look, there is still an outstanding requested change above that hasn't yet been made. But in addition to that, I think the biggest issue here is how the option is tracked: it is being tracked as a property of the parser, even though the parser has nothing to do with this behavior. It should be tracked on the Query sequence, which is the object that manages the life cycle of each Query, which is what the scope of the option is being implemented in. There is also a long-standing issue about putting any properties on the RowDataPacket prototype, as it will conflict with any row names a user may be using. Eventually the existing ones will get removed in the next major, but unless someone can clarify the reason there are new ones being added here and how there is absoltely no other way to accomplish the task, no new properties should be added to RowDataPacket prototype. I will take a closer look into these issues as well, to see what I can do to help this along, but if someone can provide details here about these, that would help speed it along as well. |
|
Awesome let us know ! I'm not sure how i can help when reading what you just said but i'm craving for this feature |
|
@dougwilson Did you get some time to take a look into that issue ? I'm not sure i'm able to provide advices or how we should do it by myself... |
|
I have not, no. |
|
@dougwilson Since this pull request is almost dead I created my own solution to this. |
dougwilson
force-pushed
the
master
branch
2 times, most recently
from
946727b
to
37fbbdd
Compare
This is for #770
You can use the rowsAsArray option(global, per query) to get the resultsets as an array type.
I hope this helps.