-
Notifications
You must be signed in to change notification settings - Fork 437
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
[RFC] Better Data Types #678
Comments
(nitpick) const result = new Buffer.alloc(8); should probably be: const result = Buffer.alloc(8); (carry on) |
Given the little I see it seems to make more sense to limit scope and try mapping many types to the fewer supported in JavaScript (rather than the opposite: supporting more types and features). The statement "cannot be converted to/from JavaScript without a loss of precision or information" would seem to suggest more time and effort than is currently expended on this project and version. Since there are 104 open issues some more than 2 years old, 21 open pull requests and 46 branches I'd suggest time constraints and quality as more relevant to this project. |
Is this being worked on? This is a very important issue for projects involving decimal values, and I was thinking about starting a prototype for this RFC. |
An example of where this would help is in the datetimeoffset type, which AFAIK right now just uses the local timezone of the server when fed a Javascript date... which completely defeats the point of having a datetimeoffset value in the first place. |
@IanChokS any updates so far? |
Hi @KAMAELUA , Unfortunately, there is no progress for now. The task is currently in the backlog and there is no one working on it yet. |
This feature would be absolutely lovely. we're working on a full rewrite of our Data Types in Sequelize (sequelize/sequelize#14505) and the way Tedious currently represents some values in JS leads to unavoidable precision loss:
We'd prefer to receive strings for all of the above types. Being able to customize their parsing would enable us to fix this on our end. |
To ride on this, would be great if tedious can provide an option to allow column returning string. |
To simplify this, could it be possible to just have an option to treat certain SQL types as binary, and just return the raw response for that column in a buffer ? We could then parse the buffer. A pluggable encoder/decoder system would be great but looks like a lot of work, and it looks like that project has stalled. Also, encoding isn't as important as decoding, SQL server does a pretty good job of casting strings to the required type when needed. I'm happy with casting to strings as needed when sending data to SQL server. If all my queries were SELECTS I'd cast the columns to binary (or string) during the select, but many result sets I deal with come from stored procedures which I don't have control over. |
inspired by issues like #608, #675, #487, #474, #163, #339, #1058 and comments made by @chdh over at #674, I want to propose the following changes to the data type system to make it more robust and extensible.
These will be breaking changes, so I'm trying to collect some feedback first. 😅
What's currently wrong with data types?
Many (but not all) of the data types supported in SQL Server do not map 1:1 to JavaScript types. This causes a lot of friction in the form of precision loss or conversion failures. E.g.
numeric
/decimal
(depending on theirprecision
andscale
),bigint
, and many of the date and time formats, cannot be converted to/from JavaScript without a loss of precision or information.There are JavaScript libraries (
big-number
,bigdecimal
,moment
,js-joda
), that implement custom versions of compatible types in JavaScript itself, but I don't want to add these libraries as dependencies oftedious
and "force" our users to use them, especially because there are often many competing libraries for the same use-case, and it's not always clear which one is the "best".How do we fix this?
I want data types to be pluggable with custom encoder/decoder functions. A encoder function would be responsible for converting a JavaScript object into it's TDS binary representation, and the decoder function would be responsible for converting the binary representation back into a JavaScript object.
tedious
would ship with a default set of encoder and decoder functions. Data Types which can be mapped to JavaScript types directly would be fully supported out-of the box, while all other data types will come with default implementations that will encode from and decode to plain JavaScript strings. This would ensure thattedious
can work with all data-types out of the box, without loss of information.Separate plug-in libraries can then provide more specific conversion functions.
Fantasy API
Here's a very rought example of how this could look like:
Other changes
While we're at it, I'd like to propose another change to how the data types API works and is used. I think we should no longer pass data type conversion errors to the Request callback, but instead throw the error right when
request.addParameter
is called. This would improve the error stack trace (it will point to exactly where the incorrect data type value was passed) and would allow us to clean some of thetedious
internals to not have to hold onto these errors. It would also allow us to change the internal storage of parameters - we'd no longer have to store the original value for the parameter, just theBuffer
result of the conversion. This'd allow V8 to perform better optimizations, as we'd move from a data structure with a megamorphicvalue
property to one wherevalue
is always a buffer.The text was updated successfully, but these errors were encountered: