-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Add JSON support in query and response #1299
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
base: master
Are you sure you want to change the base?
Conversation
Who see this commit, and want this merge (until it will accepted) you can manually install it using:
|
Readme.md
Outdated
[coveralls-url]: https://coveralls.io/r/felixge/node-mysql?branch=master | ||
[downloads-image]: https://img.shields.io/npm/dm/mysql.svg | ||
[downloads-url]: https://npmjs.org/package/mysql | ||
https://github.com/felixge/node-mysql/pull/1299 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revert the README change, or we can't actually merge your PR.
Thank you for this PR! I added a few comments and also, we need you to add tests for all the new functionality and add documentation to the README for the functionality as well. Also, your changes case many of our tests to fail, so please look into the failures and get the existing tests to pass. |
Do I need to change the version number in package.json? |
I think reverting that |
I have reverting it... didn't pushed it yet |
Now you can pass JSON object to insert/update queries, and it will stringify it. Using Example: Inserting:
Quering:
Result:
As you can see. The JSON object converted to string for storing, and back to object, when quering. |
I seems the tests are failing because of linting issues /home/travis/build/felixge/node-mysql/lib/protocol/Parser.js
301:11 error Missing semicolon semi
304:29 error Missing semicolon semi
305:43 error Missing semicolon semi
307:15 error Empty block statement no-empty
/home/travis/build/felixge/node-mysql/lib/protocol/SqlString.js
42:55 error Missing semicolon semi
47:36 error Missing semicolon semi
/home/travis/build/felixge/node-mysql/index.js
3:1 warning Missing JSDoc parameter description for 'config' valid-jsdoc
3:1 warning Missing JSDoc @returns for function valid-jsdoc
15:1 warning Missing JSDoc parameter description for 'config' valid-jsdoc
15:1 warning Missing JSDoc @returns for function valid-jsdoc
27:1 warning Missing JSDoc parameter description for 'config' valid-jsdoc
27:1 warning Missing JSDoc @returns for function valid-jsdoc
38:1 warning Missing JSDoc @returns for function valid-jsdoc
38:1 warning Missing JSDoc for parameter 'sql' valid-jsdoc
38:1 warning Missing JSDoc for parameter 'values' valid-jsdoc
38:1 warning Missing JSDoc for parameter 'callback' valid-jsdoc
48:1 warning Missing JSDoc parameter description for 'value' valid-jsdoc
48:1 warning Missing JSDoc parameter description for 'stringifyObjects' valid-jsdoc
48:1 warning Missing JSDoc parameter description for 'timeZone' valid-jsdoc
48:1 warning Missing JSDoc @returns for function valid-jsdoc
61:1 warning Missing JSDoc parameter description for 'value' valid-jsdoc
61:1 warning Missing JSDoc parameter description for 'forbidQualified' valid-jsdoc
61:1 warning Missing JSDoc @returns for function valid-jsdoc
73:1 warning Missing JSDoc parameter description for 'sql' valid-jsdoc
73:1 warning Missing JSDoc parameter description for 'values' valid-jsdoc
73:1 warning Missing JSDoc parameter description for 'stringifyObjects' valid-jsdoc
73:1 warning Missing JSDoc parameter description for 'timeZone' valid-jsdoc
73:1 warning Missing JSDoc @returns for function valid-jsdoc
95:1 warning Missing JSDoc @returns for function valid-jsdoc
95:1 warning Missing JSDoc for parameter 'className' valid-jsdoc |
Fixed |
Hi @AminaG , unfortunately this PR is not backwards-compatible, so there is no timeline when we could ever incorporate the changes. The reason is that the changes here look at all columns and sniff their string contents to see if they look like they are a JSON object and parse it, which will break a lot of people. This PR has the following two issues that need to be addressed:
|
Also, as far as I can tell, you only have tests for the escaping part; we need tests for the parsing part as well. |
What are you suggesting? You right. if a end-user for example enter in text input field for his name |
Seems this should be implemented in lib/protocol/packets/RowDataPacket.js:typeCast, the same way dates are. https://github.com/felixge/node-mysql/blob/master/lib/protocol/constants/types.js#L23 |
I can take this over, let me know. |
5847ec8
to
be37e88
Compare
65c4c0c
to
fa96a75
Compare
638d79d
to
88bade0
Compare
946727b
to
37fbbdd
Compare
there's no need to set conn's readline if CheckConnLiveness is false, and the ReadTimeout shall work with rawConn.Read inside conncheck. buffer.fill will do SetReadDeadline if needed
Now you can pass JSON object to insert/update queries, and it will stringify it.
When you run "select", it will automatically parse it to JSON object (if it success)
Using Example:
Inserting:
Quering:
Result:
As you can see. The JSON object converted to string for storing, and back to object, when quering.