Skip to content

Conversation

@ciaranj
Copy link
Contributor

@ciaranj ciaranj commented Dec 30, 2011

My database appears to hit all your (presumably) known issues :)

Please find attached a pull request to add initial support for nvarchar(max) columns. I've implemented NULL and known length, but not 'unknown length' (I couldn't seem to get a query to generate column results like that.)

On a side note, is there not a fundamental issue with the node.js buffers for these large (64bit length) types of data ? Just not sure
how one would cope with that ?

I've extended this pull request to also include support for binary(n) and varbinary(n/max) types (the results are returned as a byte array, fixed width in the former case, variable width in the latter.)

I've extended it (again) to support 'decimal(s,p)' column types, these appear to be synonymous to numeric(s,p) types so I've piggy-backed on that implementation, I note that it won't support the full width of data that could be returned, I guess a full big integer library support is required after all :(

Provides a partial solution for handling of Partially Length-prefixed Byte
data.  Does not currently support 'unknown plp length' data (solely because
I struggled to get a query to generate that type)

Not really sure how one would support these types fully as I would imagine they
break the nodejs buffer limits before we can return all the data we need (the buffer.position is 32bit,
the maximum returnable data is around 64bit )
Utilises the same processing logic as the NumericN type.
However I think this (and the existing) logic does not fully support
the numeric range that could come back from the server  i.e.
if precision  >=29 or precission <=38 then we need to be operating over
16 bytes, we definitely aren't :)
@pekim
Copy link
Collaborator

pekim commented Dec 31, 2011

I made a rather large change to token parsing (f550296), that will make it somewhat difficult to do a git merge for these changes. So I think that I'm going to tackle them one data type at a time.

For each data type, I'll take the gist of your changes, and apply them manually.

@ciaranj
Copy link
Contributor Author

ciaranj commented Dec 31, 2011

No worries I separated out the test commits so you should be able to cherry
pick them all out first, then try the other ones (sorry)

  • cj

On Saturday, December 31, 2011, Mike D Pilsbury <
reply@reply.github.com>
wrote:

I made a rather large change to token parsing
(f550296), that will make it
somewhat difficult to do a git merge for these changes. So I think that I'm
going to tackle them one data type at a time.

For each data type, I'll take the gist of your changes, and apply them
manually.


Reply to this email directly or view it on GitHub:
#3 (comment)

@pekim
Copy link
Collaborator

pekim commented Dec 31, 2011

pekim/tedious/@d47f645a8553f9e6210d01ce383c383fb940ac55 - Added support for decimal type, including 12 and 16 byte numeric/decimal values

@ciaranj
Copy link
Contributor Author

ciaranj commented Jan 1, 2012

Cool, so thats decimal support, I'm surprised that the >8byte wide values 'work' and don't truncate on the precision, why is that?

@pekim
Copy link
Collaborator

pekim commented Jan 1, 2012

12 and 16 byte precision values do indeed appear to work.

I'm taking 32 bits at a time, using Buffer.readUInt32LE to create a Javascript number. The 2, 3 or 4 of those Numbers (for 8, 12, or 16 bytes) are multiplied together to make a (possibly slightly inaccurate) Number for the precison value.

Although there may be some loss of accuracy due to the imprecision of Javascript floating point numbers, even after scaling, the result seems to be correct for the numbers I happen to have used in the tests. I'm sure that some values won't be so accurately represented, so there may sometimes be small differences between the value returned from the server and the value represented in Javascript.

pekim added a commit that referenced this pull request Jan 1, 2012
pekim added a commit that referenced this pull request Jan 1, 2012
pekim added a commit that referenced this pull request Jan 1, 2012
@pekim
Copy link
Collaborator

pekim commented Jan 1, 2012

I think that I've now covered everything.

@pekim pekim closed this Jan 1, 2012
@ciaranj
Copy link
Contributor Author

ciaranj commented Jan 1, 2012

Looks good. I just need/want Float and I'm good to export all my data outta SQL Server (my end-game!) .. I suspect for the varbinary(max) and nvarchar(max) it would be preferable to write out to a temp file and return a stream onto that (but for my usages I'm good I think ;) ) ... I'm still unsure how one would cope with resultsets > 2GB using a node.js buffer object (but I probably need to revise them!)

@pekim
Copy link
Collaborator

pekim commented Jan 2, 2012

I've added support for real and float.
See issue #5.

@ciaranj
Copy link
Contributor Author

ciaranj commented Jan 2, 2012

Looks good, thank you :) I can now happily return all the data types I need! Woohoo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants