Skip to content

Fixed conversion of LocalDateTime and LocalTime to String in Bulk Copy #1640

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

Merged
merged 5 commits into from
Nov 5, 2021

Conversation

joechu1
Copy link
Contributor

@joechu1 joechu1 commented Aug 13, 2021

When writing a LocalDateTime with bulk copy, the driver may return an error when second-of-minute and nano-of-second is 0:

com.microsoft.sqlserver.jdbc.SQLServerException: Conversion failed when converting date and/or time from character string. Msg 241, Level 16, State 1, Conversion failed when converting date and/or time from character string.

When time units to the right of the minute are all 0's, the LocalDateTime.toString() will simplify the string format to uuuu-MM-dd'T'HH:mm, which SQL Server is not able to process due to the missing seconds. This PR changes the string conversion to use the DateTimeFormatter and ensures that it includes the necessary padding.

For example:

LocalDateTime                  = 2021-08-13 00:00:00

Current - toString()           = 2021-08-13T00:00              // Returns error
New     - DateTimeFormatter    = 2021-08-13T00:00:00           // It works!

Note that this is very similar to PR #1485.

Copy link
Collaborator

@David-Engel David-Engel left a comment

Choose a reason for hiding this comment

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

We should also extend testBulkCopyDateTimePrecision added in #1559 to include data that this fix is covering.

AbductedNoah
AbductedNoah previously approved these changes Oct 28, 2021
@lilgreenbird
Copy link
Contributor

hi @joechu1
Thank you for your contribution, apologies this took so long, we were busy with other higher priorities issues.

@lilgreenbird lilgreenbird merged commit d0de2c5 into microsoft:dev Nov 5, 2021
@lilgreenbird lilgreenbird added this to the 9.5.0 milestone Nov 5, 2021
@lilgreenbird lilgreenbird changed the title Fix conversion of LocalDateTime and LocalTime to String in Bulk Copy Fixed conversion of LocalDateTime and LocalTime to String in Bulk Copy Nov 5, 2021
@lilgreenbird lilgreenbird self-assigned this Dec 1, 2021
@lilgreenbird lilgreenbird modified the milestones: 9.5.0, 9.4.1 Hotfix Dec 1, 2021
lilgreenbird pushed a commit to lilgreenbird/mssql-jdbc that referenced this pull request Dec 1, 2021
…icrosoft#1640)

* Use DateTimeFormatter to convert LocalDateTime and LocalTime to String

* Streamlining toString calls and adding tests

* Changed string pattern to match expected precision

* Formatting

* Formatting

Co-authored-by: Joe Chu <5282594+joechu1@users.noreply.github.com>
Co-authored-by: David Engel <v-davidengel@microsoft.com>
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.

None yet

4 participants