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

Finish support for RFC4180 for CSV bulk insert operations #2338

Merged
merged 2 commits into from Apr 1, 2024

Conversation

funkyjive
Copy link
Contributor

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
Contributor Author

funkyjive commented Feb 22, 2024

@microsoft-github-policy-service agree

@lilgreenbird
Copy link
Member

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

@lilgreenbird lilgreenbird added this to In progress in MSSQL JDBC via automation Feb 22, 2024
@lilgreenbird lilgreenbird added the Enhancement An enhancement to the driver. Lower priority than bugs. label Feb 28, 2024
@Jeffery-Wasty Jeffery-Wasty moved this from In progress to Under Peer Review in MSSQL JDBC Feb 29, 2024
barryw-mssql
barryw-mssql previously approved these changes Mar 21, 2024
Copy link
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
Copy link
Member

@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

MSSQL JDBC automation moved this from Under Peer Review to In progress Mar 26, 2024
@lilgreenbird lilgreenbird moved this from In progress to Under Peer Review in MSSQL JDBC Mar 26, 2024
tkyc
tkyc previously approved these changes Mar 26, 2024
MSSQL JDBC automation moved this from Under Peer Review to In progress Mar 26, 2024
Copy link
Member

@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
Copy link
Contributor Author

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.

Copy link

codecov bot commented Mar 27, 2024

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
17 checks passed
MSSQL JDBC automation moved this from In progress to Closed/Merged PRs Apr 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement An enhancement to the driver. Lower priority than bugs.
Projects
MSSQL JDBC
  
Closed/Merged PRs
Development

Successfully merging this pull request may close these issues.

None yet

5 participants