You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This package generally works great, but I have run in to a significant problem with the limitations of the JavaScript Date objects used by the package as output for SQL DateTime and TimeStamp values.
The JavaScript Date format cannot represent anything before 1970, even though the SQL Date and DateTime can. Also, TimeStamps can have the value of 0000-00-00 00:00:00. Round tripping in a database application is severely hampered by the fact that the JavaScript Date cannot hold most of the values that MySQL can represent.
The mysql package solved the problem by having a dateStrings: true option, and I think it can be solved similarly here. I already wrote some code to fix it, but I want your opinion on the best way to pass a dateStrings option through to common.js where it is best replaced.
I fixed it by replacing the new Date(...) calls in common.js with calls to two new functions, that can either call new Date(...) and return it, mimicking the old behavior, or build a string out of the value, like one that an sql select statement would return, or that the node mysql package would return if dateStrings: true is set. It works great, and seems to entirely fix the problem, but there is still the matter of relaying the dateStrings option to exports.readMysqlValue in common.js.
Do you have a preferred way to relay dsn.dateStrings (or perhaps options.dateStrings if you prefer) to readMysqlValue in the common.js file? It could be done as a function argument, with a setter in the file, or some other way.
Thanks,
R.F.
The text was updated successfully, but these errors were encountered:
Those are good points on the limitations of the JS Date object. The mysql package probably has an option for 64-bit ints that don't fit in JS numbers either.
The emitter argument of readMysqlValue in common.js receives the zongji instance and then the connection details can be accessed from the ctrlConnection property according to these rules.
You'll want to rename that argument, probably to zongji since it's no longer just for emitting errors. Also, it needs to be renamed where called in rows_event.js. To test, you'll have to modify the relevant type tests to accept different results based on the differing connection details.
I just added a pull request fixing this #71 . It passes all the tests that I can run with my version of MariaDB, both with and without the dateStrings option. MariaDB does not support the built in JSON objects available in very recent versions of MySQL yet, so I can't run those. The type tests usefully check the code with and without the dateStrings option without modification due to the fact that they interpret symmetrically on both ends of the equality statements.
Your input was really useful here, and without that I think it would have taken much longer to figure out how to read the option cleanly. I wanted others to have the benefit of that, so I put a few useful notes in the code to make it easier for people in the future.
This package generally works great, but I have run in to a significant problem with the limitations of the JavaScript Date objects used by the package as output for SQL DateTime and TimeStamp values.
The JavaScript Date format cannot represent anything before 1970, even though the SQL Date and DateTime can. Also, TimeStamps can have the value of 0000-00-00 00:00:00. Round tripping in a database application is severely hampered by the fact that the JavaScript Date cannot hold most of the values that MySQL can represent.
The mysql package solved the problem by having a dateStrings: true option, and I think it can be solved similarly here. I already wrote some code to fix it, but I want your opinion on the best way to pass a dateStrings option through to common.js where it is best replaced.
I fixed it by replacing the new Date(...) calls in common.js with calls to two new functions, that can either call new Date(...) and return it, mimicking the old behavior, or build a string out of the value, like one that an sql select statement would return, or that the node mysql package would return if dateStrings: true is set. It works great, and seems to entirely fix the problem, but there is still the matter of relaying the dateStrings option to exports.readMysqlValue in common.js.
Do you have a preferred way to relay dsn.dateStrings (or perhaps options.dateStrings if you prefer) to readMysqlValue in the common.js file? It could be done as a function argument, with a setter in the file, or some other way.
Thanks,
R.F.
The text was updated successfully, but these errors were encountered: