Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Support SQL Server 2008 Datatypes #244

Closed
metaskills opened this issue Nov 22, 2015 · 24 comments
Closed

Support SQL Server 2008 Datatypes #244

metaskills opened this issue Nov 22, 2015 · 24 comments

Comments

@metaskills
Copy link
Contributor

This work needs to happen once we support FreeTDS 0.95 via #233. Here are the data types we want to support.

  • [date]
  • [datetime2]
  • [datetimeoffset]
  • [time].

I created a new thread on FreeTDS' list. The new one is titled Compiling Against FreeTDS 0.95 w/7.3 TDSVER Features Possible?. A reply is below.

The binary structure is well defined (and an ABI actually) however is
defined in tds.h (TDS_DATETIMEALL). The date field is the same as
dtdays in DATETIME however
time is completely different (100-nanosecondth from start of day).

dbdatecrack only supports DATETIME. I suppose a new dbanydatecrack (or whatever)
extension could be defined (possibly extending time precision). Or in
the meantime you could pass date field into a DATETIME, use
dbdatecrack and split time manually (at least the precision).

cc @saurabh500
@metaskills
Copy link
Contributor Author

Linked adapter issue - rails-sqlserver/activerecord-sqlserver-adapter#401

@metaskills metaskills mentioned this issue Nov 22, 2015
Merged
@metaskills metaskills added this to the 1.0.0 milestone Nov 22, 2015
@metaskills
Copy link
Contributor Author

This commit just happened in master, which means this will be in FreeTDS v0.99 @freddy77 says this is the start and it looks good to me. I wonder once we get that last 0.95 bug addressed and #233 finished, if we can add a new env var to test latest master branch of FreeTDS.

FreeTDS/freetds@a35fe7e

@metaskills metaskills mentioned this issue Nov 22, 2015
Closed
@metaskills
Copy link
Contributor Author

@saurabh500 After you have time to see that commit in FreeTDS master, will we be able to use the binary data returned across the wire in FreeTDS 0.95 or will we have to use 0.99 and the new stuff Freddy is doing? TIA

@freddy77
Copy link

I pushed some changes to master to integrate dbanydatecrack. This function use new DBDATEREC2 type which change milliseconds to nanoseconds in the DBDATEREC structure and accepts an additional type parameter (which can be returned by dbcoltype) for the type.
So you can use dbdata+dbcoltype+dbanydatecrack to parse any date/time type (even future BIGTIME or whatever). The idea is to backport these changes in 0.95 too.
The function returns FAIL on failure (currently the type is not a date/time or you are passing some NULL parameters).
ABI will change from 5.0 to 5.1 to mark this change.

Could you try if works for you? I extended a test and is actually working.

@metaskills
Copy link
Contributor Author

Thanks! I just setup our build scripts to allow 0.99 and/or FreeTDS at GitHub's master branch. The GitHub download and build fails tho. I suspect something on my end. However, I can build freetds-dev.0.99.469 on the FTP site.

When will master update the dev 0.99 version? If tomorrow, I can do it then but I think I need the FTP version updated.

@metaskills
Copy link
Contributor Author

OK, freetds-dev.0.99.470 had a this first commit from a few days ago. But still waiting for a few others to show up in the next dev build. Will play with this tomorrow if 471 comes out. Freddy, I assume those are automated?

@freddy77
Copy link

Yes. At 9 utc if I remember
Il 23/Nov/2015 20:03, "Ken Collins" notifications@github.com ha scritto:

OK, freetds-dev.0.99.470 had a this first commit from a few days ago. But
still waiting for a few others to show up in the next dev build. Will play
with this tomorrow if 471 comes out. Freddy, I assume those are automated?

—
Reply to this email directly or view it on GitHub
#244 (comment)
.

@freddy77
Copy link

They are already in the last snapshot.

Frediano

2015-11-23 20:03 GMT+00:00 Ken Collins notifications@github.com:

OK, freetds-dev.0.99.470 had a this first commit from a few days ago. But
still waiting for a few others to show up in the next dev build. Will play
with this tomorrow if 471 comes out. Freddy, I assume those are automated?

—
Reply to this email directly or view it on GitHub
#244 (comment)
.

@metaskills metaskills mentioned this issue Nov 26, 2015
Merged
@metaskills
Copy link
Contributor Author

OK... I have started this PR #245 for the work. So far I have [date] and [time] working and fully tested. Should not take me long at all to get [datetime2] and [datetimeoffset] working too.

@freddy77 I'll send another message when this is done, but I think this is looking great to get into 0.95.

@metaskills
Copy link
Contributor Author

@saurabh500 ☝️

@metaskills
Copy link
Contributor Author

@freddy77 So far everything looks great. However, the SYBMSDATETIMEOFFSET structure always returns integers like 32653 or 32652 for tzone. It appears this is not implement yet?

@freddy77
Copy link

Well... this field was never implemented by nobody actually... Microsoft does not define it and Sybase does not use!
I don't know which should be the range of this field. The only documentation (Sybase) is 0-127 which does not make any sense to me... but as I said is unused by Sybase either so we could define it as we want.
I suppose the minute offset (as datetimeoffset does) would be fine.

@freddy77
Copy link

And what value should have if not a datetimeoffset ?? 0 ??

@metaskills
Copy link
Contributor Author

Microsoft does not define

@freddy77 Perhaps @saurabh500 can help us here? The only thing I know is that the Microsoft [datetimeoffset] documentation mentions that the zone offset range is -14:00 through +14:00. So if that were minutes, I guess if the MS structure returned anything for -840 to 840 minutes for the tzone would be fine?

Saurabh, any thoughts?

@freddy77
Copy link

What waste of bits :-)
Sounds good to me. I'll have a look at tm structure too.
What should I set for types that does non provide the offset?
Il 27/Nov/2015 19:03, "Ken Collins" notifications@github.com ha scritto:

Microsoft does not define

@freddy77 https://github.com/freddy77 Perhaps @saurabh500
https://github.com/saurabh500 can help us here? The only thing I know
is that the Microsoft [datetimeoffset] documentation
https://msdn.microsoft.com/en-us/library/bb630289.aspx# mentions that
the zone offset range is -14:00 through +14:00. So if that were minutes, I
guess if the MS structure returned anything for -840 to 840 minutes for
the tzone would be fine?

Saurabh, any thoughts?

—
Reply to this email directly or view it on GitHub
#244 (comment)
.

@metaskills
Copy link
Contributor Author

What waste of bits :-)

LOL

What should I set for types that does non provide the offset?

Maybe 0 ¯\_(ツ)_/¯

@saurabh500
Copy link

@metaskills -14:00 thru +14:00 sounds good.
In case the offset is not provided, a value of 0 should be good.
There are two bytes used to represent the offset
(bytes[length - 2] + (bytes[length - 1] << 8))
On the client side, this would automatically result in a 0 value of the tzone offset.

@saurabh500
Copy link

@freddy77 and @metaskills

My understanding is the FreeTDS will return the coltype for the data types being supported in this issue.
https://github.com/rails-sqlserver/tiny_tds/blob/master/ext/tiny_tds/result.c#L203

If it is any of the 2008 types, then TinyTDS will call dbconvert(...) to get the bytes from FreeTDS parser.

FreeTDS will return the a copy of the bytes from the TDS stream and TinyTDS will attempt to create the correct datatype from these bytes?

Hence this will allow a separation of concerns where FreeTDS will handle the stream of bytes and the logical data structure will be handled by TinyTDS.

Let me know if I got this correct.

@metaskills
Copy link
Contributor Author

then TinyTDS will call dbconvert(...)

@saurabh500 Correct till that point. I made the decision awhile back ago that I would wait till FreeTDS supported these types rather than using convert since that would result in loss of precisions and info for some types. That's OK tho, @freddy77 has added these new types - notably the struct used with the DBLIB extensions dbanydatecrack. Here is the current work in freetds_current branch where we have all of these except the [datetimeoffset] type. https://github.com/rails-sqlserver/tiny_tds/blob/freetds_current/ext/tiny_tds/result.c#L287-L317

Just waiting on Freddy to get that zone info in. Once that is done, he has said that he will apply the work for these 2008 types into FreeTDS 0.95.

@freddy77
Copy link

freddy77 commented Dec 1, 2015

I think you refer to this for datetimeoffset FreeTDS/freetds@c6686aa

@saurabh500
Copy link

notably the struct used with the DBLIB extensions dbanydatecrack.

@metaskills dbanydatecrack handling the bytes is a good proposition. +1 for that.

@freddy77 the change looks good.

@metaskills
Copy link
Contributor Author

Sorry for the late reply. I finished the [datetimeoffset] type implementation.

  • v0.99.479 - ALL GREEN!
  • v0.95.73 - BUG datetimeoffset tzone

@freddy77 I think 0.95 latest has a bug in it. The tds_microsoft_dbdaterec2.tzone returned value for my test came back as 32692 vs -480. I also noticed the comments for each in sybdb.h appear to be wrong.

@metaskills metaskills mentioned this issue Dec 7, 2015
Closed
@metaskills
Copy link
Contributor Author

Closing this issue. Pull request #245 is about to land!

@metaskills
Copy link
Contributor Author

Work is now in master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@metaskills @freddy77 @saurabh500