-
Notifications
You must be signed in to change notification settings - Fork 428
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
Fix | Added ISQLServerBulkData to remove implementation details from ISQLServerBulkRecord #1099
Conversation
… from ISQLServerBulkRecord Fixes microsoft#1052
@cheenamalhotra please take a look, thanks |
Looks like the test failure is spurious - hit a deadlock |
Hi @jonfreedman, |
src/test/java/com/microsoft/sqlserver/jdbc/bulkCopy/BulkCopyTestUtil.java
Outdated
Show resolved
Hide resolved
src/main/java/com/microsoft/sqlserver/jdbc/ISQLServerBulkData.java
Outdated
Show resolved
Hide resolved
Looks like there's some conflicts, but other than that, the PR looks good to me. |
@peterbae I have merged from upstream |
Thanks @jonfreedman, I also noticed that we have:
in BulkCopyCSVTest.java line 169. Could you remove the cast to ISQLServerBulkRecord? |
@peterbae I've removed the cast, didn't find any others |
Can this be merged? |
Hi @jonfreedman, the team is contemplating whether to merge this for the next preview or not. We're trying to decide whether we want to apply a bandaid fix or just break the public apis and change the interfaces completely. We will merge/close this PR after discussions. |
It's not really clear to me why you can't do both. You can see that this pr has been open for 3 months, based on an issue from 5 months ago. At this point all it really accomplishes is that I can delete some code that I've been running in production for almost 6 months. Are you saying that if you don't merge this then the next release will be rewritten with breaking changes, or that the 'fix' will happen at some later date? |
Hi @jonfreedman, |
That's great, please go ahead and change whatever you need to |
Hi @jonfreedman, |
Added @deprecated tag to ISQLServerBulkRecord
The PR is merged. Thanks for the contribution @jonfreedman. |
* Release | Update SNAPSHOT for 7.5.0-preview release (#1129) * Release | Update driver version * Release| Add pom and gradle files * Fix | Remove extra spaces in SQLServerDatabaseMetaData.getNumericFunctions()/SQLServerDatabaseMetaData.getStringFunctions() return values (#1117) * Tests | Improve assertions in ComparisonUtil (#1100) * ComparisonUtil : replace assertTrue with assertEquals Failure message should include the actual/expected values and that is what assertEquals does. * Corrected BigDecimal comparison Scale difference should be ignored. * Fix | Add list of trusted endpoints for AKV (#1130) * added list of trusted endpoints * rename * Update issue templates (#1148) * Fix getImportedKeys() returning duplicate rows if multiple FKs have the same name (#1092) * fixed for github #1091 dup rows * Fix STAsBinary returning null for a single point (#1074) * populate wkb for point * fix stasbinary issue * change variable name * apply same variable name change * Performance | Improved performance of column name string lookups (#1066) * Fix | Made column name lookup more performant * Fix | Spacing * Fix | Changed arraylist to maps * Fix | Add newline * Add | Caching for previously retrieved columns * Fix | Formatting * Fix | Variable issue * Fix | null order * Fix | part 2 * Revert | Integer changes * Fix | Trim retrieved column name * Fix | Clear column names whenever ResultSet cursor is moved * Revert "Fix | Clear column names whenever ResultSet cursor is moved" This reverts commit db42d44. * Add | Implement hashCode() and equals() APIs for SQLServerDataTable and SQLServerDataColumn (#1146) * Add | Add hashCode()/equals() methods to SQLServerDataColumn and SQLServerDataTable * Fix issue with truststore password being removed too early for XA transaction (#1133) Fix | Fix issue with truststore password being removed too early for XA transaction * Fix | SQLServerDatabaseMetada.getColumns not escaping wildcard (#1138) * Feature | Introduce JAVA 13 Support (#1151) * JDK 13 | Remove jre12 from pom file * JDK 13 | Update Gradle build file * JDK 13 | Update Azure-pipelines * Test | Updated SQL Server from 2008R2 to 2012 in CI (#1153) * Performance | Disabled pattern matching when using CallableStatements and SQLServerParameterMetaData (#1149) * Fix | Added ISQLServerBulkData to remove implementation details from ISQLServerBulkRecord (#1099) * Feature | Added support for Always Encrypted with Secure Enclaves (#1155) * Release | 8.1.0-preview release changes (#1158)
New ISQLServerBulkData interface which extracts the read-only methods from ISQLServerBulkRecord
Fixes #1052