Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add support for datetimeoffset data type #26
Add support for datetimeoffset data type #26
Changes from 3 commits
cdaa9d5
3260e34
25b0b06
1dfb84c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Shouldn't this be 10 bytes? Why 20?
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.
I'm not sure, but I have a theory. I'd like to air it here; please let me know if this sounds completely bonkers.
I couldn't find anything in the documentation that states the size of the
TIMESTAMPOFFSET_STRUCT
, so I added the sizes of the constituent elements of the struct together. 3 SQLSMALLINT + 5 SQLUSMALLINT + 1 SQLUINTEGER. I used the conversion table in https://docs.microsoft.com/en-us/sql/odbc/reference/appendixes/c-data-types to translate those to C types, and from there I guessed at the size of each, based on https://en.wikipedia.org/wiki/C_data_types. I guessed thatshort int
andunsigned short int
would both be 2 bytes (the minimum size) andunsigned long int
would be 4 bytes (again, the minimum size). That's 3 * 2 + 5 + 2 + 1 * 4 = 20.I've noticed that https://docs.microsoft.com/en-us/sql/t-sql/data-types/datetimeoffset-transact-sql states that the storage size of the
datetimeoffset
column type is 10 bytes, but I suppose that that's because it uses a more compact data format (something like nanoseconds since some zero date, plus the time zone offset...).At a more pragmatic level, the code works when I allocate 20 bytes. If I try to allocate 10 bytes, it doesn't work:
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.
Fair enough. 👍 If you can document that somewhere, then the code will be self-explanatory. Allocating 20 bytes isn't a big deal more than 10, I was just curious why it differed to the claim I saw in MSDN.
Note to self, fix that error reporting. It's truncated intentionally, but out of laziness.
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.
Indeed, I've now added a comment in a new commit.
FWIW, I had in mind doing that all along, but I felt the need to run the above explanation by someone first, as I wasn't entirely sure that it was correct.