-
Notifications
You must be signed in to change notification settings - Fork 431
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
JSON datatype support #2558
base: main
Are you sure you want to change the base?
JSON datatype support #2558
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2558 +/- ##
============================================
- Coverage 51.01% 50.09% -0.93%
+ Complexity 3921 3829 -92
============================================
Files 147 147
Lines 33483 33544 +61
Branches 5609 5619 +10
============================================
- Hits 17081 16803 -278
- Misses 13991 14347 +356
+ Partials 2411 2394 -17 ☔ View full report in Codecov by Sentry. |
src/test/java/com/microsoft/sqlserver/jdbc/unit/statement/RegressionTest.java
Show resolved
Hide resolved
/azp run public-mssql-jdbc.windows |
Azure Pipelines successfully started running 1 pipeline(s). |
No pipelines are associated with this pull request. |
/azp run public-mssql-jdbc.linux |
/azp run CI-MacOS |
Azure Pipelines successfully started running 1 pipeline(s). |
1 similar comment
Azure Pipelines successfully started running 1 pipeline(s). |
src/test/java/com/microsoft/sqlserver/jdbc/callablestatement/CallableStatementTest.java
Outdated
Show resolved
Hide resolved
…m/microsoft/mssql-jdbc into user/divang/json-datatype-support
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to add tests for datatype conversions. See PR for sql_variant for example
@@ -150,6 +150,37 @@ public void testBulkCopyDateTimePrecision() throws SQLException { | |||
} | |||
} | |||
|
|||
@Test | |||
public void testBulkCopyJSON() throws SQLException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before this merges, just a heads up and reminder that both internal and public pipelines will need the new SQL Server version that supports JSON.
Can you update the description with a list of high level areas that have been modified in this PR ? |
|
||
JSON(SSType.Category.JSON, EnumSet.of(JDBCType.Category.CHARACTER, JDBCType.Category.LONG_CHARACTER, | ||
JDBCType.Category.CLOB, JDBCType.Category.NCHARACTER, JDBCType.Category.LONG_NCHARACTER, | ||
JDBCType.Category.NCLOB, JDBCType.Category.BINARY, JDBCType.Category.LONG_BINARY, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does it mean to have a LONG_BINARY on Json ? I see the other types using it as well, but wondering what its purpose is.
@@ -970,7 +985,7 @@ boolean isBinary() { | |||
* @return true if the JDBC type is textual | |||
*/ | |||
private final static EnumSet<Category> textualCategories = EnumSet.of(Category.CHARACTER, Category.LONG_CHARACTER, | |||
Category.CLOB, Category.NCHARACTER, Category.LONG_NCHARACTER, Category.NCLOB); | |||
Category.CLOB, Category.NCHARACTER, Category.LONG_NCHARACTER, Category.NCLOB, Category.JSON); //FIXME: JSON is textual? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you have a //fixme comment. Was this resolved? how does textualCategories impact APIs ?
src/main/java/com/microsoft/sqlserver/jdbc/SQLServerParameterMetaData.java
Show resolved
Hide resolved
@divang please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
…t, and fixed existing scenarios when enabling escapeDelimiter.
To add support for the new JSON datatype, the following changes have been made to the codebase: