Skip to content

Finish support for RFC4180 for CSV bulk insert operations#2338

Merged
lilgreenbird merged 2 commits intomicrosoft:mainfrom
funkyjive:RFC4180_MultiLine_Fields
Apr 1, 2024
Merged

Finish support for RFC4180 for CSV bulk insert operations#2338
lilgreenbird merged 2 commits intomicrosoft:mainfrom
funkyjive:RFC4180_MultiLine_Fields

Conversation

@funkyjive
Copy link
Copy Markdown
Contributor

@funkyjive funkyjive commented Feb 22, 2024

RFC4180 specifies support for fields that contain newlines as long as they are quoted. See section 6 in the specification here: https://www.ietf.org/rfc/rfc4180.txt

This PR enhances the read operation when the option escapeColumnDelimitersCSV is set to true to allow newline characters to survive the CSV parse process and find their way into the record. An existing unit test was enhanced to test the behavior.

@funkyjive
Copy link
Copy Markdown
Contributor Author

funkyjive commented Feb 22, 2024 •
edited
Loading

@microsoft-github-policy-service agree

@lilgreenbird
Copy link
Copy Markdown
Contributor

lilgreenbird commented Feb 22, 2024

thank you for your contribution, the team will take a look at this when time permits

@lilgreenbird lilgreenbird added the Enhancement An enhancement to the driver. Lower priority than bugs. label Feb 28, 2024
barryw-mssql
barryw-mssql previously approved these changes Mar 21, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@barryw-mssql barryw-mssql left a comment

Choose a reason for hiding this comment

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

Appears to handle newline in Mac, Linux, and Windows formats

Jeffery-Wasty
Jeffery-Wasty previously approved these changes Mar 21, 2024
View reviewed changes
lilgreenbird
lilgreenbird reviewed Mar 26, 2024
View reviewed changes
Comment thread src/test/resources/BulkCopyCSVTestInputDelimiterEscape.csv Outdated
lilgreenbird
lilgreenbird suggested changes Mar 26, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@lilgreenbird lilgreenbird left a comment

Choose a reason for hiding this comment

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

please use file formatter to format this PR

tkyc
tkyc previously approved these changes Mar 26, 2024
View reviewed changes
lilgreenbird
lilgreenbird suggested changes Mar 26, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@lilgreenbird lilgreenbird left a comment

Choose a reason for hiding this comment

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

please use file formatter to format this PR all files need to be formatted before merge

@funkyjive funkyjive dismissed stale reviews from tkyc, Jeffery-Wasty, and barryw-mssql via 32656a6 March 27, 2024 03:20
@funkyjive
Copy link
Copy Markdown
Contributor Author

funkyjive commented Mar 27, 2024

please use file formatter to format this PR all files need to be formatted before merge

I think I have what you are looking for now.

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 27, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 45.30744% with 169 lines in your changes are missing coverage. Please review.

❗ No coverage uploaded for pull request base (main@ba88da8). Click here to learn what that means.

❗ Current head f21c9f1 differs from pull request most recent head 32656a6. Consider uploading reports for the commit 32656a6 to get more accurate results

Files Patch % Lines
...soft/sqlserver/jdbc/SQLServerDatabaseMetaData.java 8.62% 53 Missing ⚠️
...microsoft/sqlserver/jdbc/SQLServerMSAL4JUtils.java 0.00% 41 Missing ⚠️
...a/com/microsoft/sqlserver/jdbc/SQLServerError.java 32.14% 17 Missing and 2 partials ⚠️
.../microsoft/sqlserver/jdbc/SQLServerConnection.java 65.30% 8 Missing and 9 partials ⚠️
...microsoft/sqlserver/jdbc/SQLServerInfoMessage.java 44.44% 14 Missing and 1 partial ⚠️
...m/microsoft/sqlserver/jdbc/SQLServerException.java 60.00% 5 Missing and 3 partials ⚠️
...lserver/jdbc/PersistentTokenCacheAccessAspect.java 20.00% 3 Missing and 1 partial ⚠️
...oft/sqlserver/jdbc/SQLServerBulkCSVFileRecord.java 80.00% 3 Missing and 1 partial ⚠️
...m/microsoft/sqlserver/jdbc/SQLServerStatement.java 71.42% 2 Missing and 2 partials ⚠️
...t/sqlserver/jdbc/SQLServerConnectionPoolProxy.java 50.00% 1 Missing ⚠️
... and 3 more
Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #2338   +/-   ##
=======================================
  Coverage        ?   50.10%           
  Complexity      ?     3825           
=======================================
  Files           ?      145           
  Lines           ?    33318           
  Branches        ?     5647           
=======================================
  Hits            ?    16695           
  Misses          ?    14233           
  Partials        ?     2390

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lilgreenbird lilgreenbird added this to the 12.7.0 milestone Mar 27, 2024
@lilgreenbird lilgreenbird merged commit 662a266 into microsoft:main Apr 1, 2024
@Eng-Fouad Eng-Fouad mentioned this pull request Aug 1, 2024
Merged
@machavan machavan mentioned this pull request Jan 6, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

4 more reviewers

@lilgreenbird lilgreenbird lilgreenbird approved these changes

@Jeffery-Wasty Jeffery-Wasty Jeffery-Wasty approved these changes

@tkyc tkyc tkyc approved these changes

@barryw-mssql barryw-mssql barryw-mssql approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Enhancement An enhancement to the driver. Lower priority than bugs.

Projects

MSSQL JDBC
Status: Closed/Merged PRs

Milestone

12.7.0

Development

Successfully merging this pull request may close these issues.

5 participants

@funkyjive @lilgreenbird @Jeffery-Wasty @tkyc @barryw-mssql