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.

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

Fixup/sqlserver datetime2 #885

Merged
merged 6 commits into from
Jan 27, 2025
Merged

Conversation

detule
Copy link
Collaborator

@detule detule commented Jan 25, 2025

Closes #793

This is a long standing issue where we were forcing a precision of 3 for all DATETIME types despite some ( for example DATETIME2 ) having potentially greater precision.

Requires a backport from nanodbc.

@detule detule requested review from hadley and simonpcouch January 25, 2025 20:39
Copy link
Collaborator

@simonpcouch simonpcouch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't have the skillset to review src/ changes confidently but see no reason not to approve from my perspective. Thanks!

describe_parameters(param_index);
const SQLSMALLINT& param_type = param_descr_data_.at(param_index).type_;
NANODBC_ASSERT(param_type < static_cast<SQLULEN>(std::numeric_limits<short>::max()));
return static_cast<unsigned long>(param_type);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may just be my C++ naïveté—apologies if so. From what I can tell, the function declares it returns a short but can actually return an unsigned long. Is this ever an issue?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call and thank you for taking a look!

The compiler will clean that up and cast back to short, but its poor form on my part. Fixed!

@detule detule merged commit 6b5635c into r-dbi:main Jan 27, 2025
16 of 17 checks passed
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.

Determine DATETIME2 precision based on parameter description
2 participants