Skip to content

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

Merged
lilgreenbird merged 5 commits intomicrosoft:devfrom
joechu1:bulkcopy-localdatetime
Nov 5, 2021
Merged

Fixed conversion of LocalDateTime and LocalTime to String in Bulk Copy#1640
lilgreenbird merged 5 commits intomicrosoft:devfrom
joechu1:bulkcopy-localdatetime

Conversation

@joechu1
Copy link
Copy Markdown
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
Copy Markdown
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.

Comment thread src/main/java/com/microsoft/sqlserver/jdbc/SQLServerBulkCopy.java Outdated
Comment thread src/main/java/com/microsoft/sqlserver/jdbc/SQLServerBulkCopy.java
AbductedNoah
AbductedNoah previously approved these changes Oct 28, 2021
@lilgreenbird
Copy link
Copy Markdown
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 <[email protected]>
Co-authored-by: David Engel <[email protected]>
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.

4 participants