Fix creating SQLServerDataTable being O(n^2) issue#514
Fix creating SQLServerDataTable being O(n^2) issue#514peterbae merged 6 commits intomicrosoft:devfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #514 +/- ##
============================================
+ Coverage 46.51% 46.56% +0.04%
+ Complexity 2218 2213 -5
============================================
Files 108 108
Lines 25312 25304 -8
Branches 4181 4176 -5
============================================
+ Hits 11775 11782 +7
+ Misses 11501 11494 -7
+ Partials 2036 2028 -8
Continue to review full report at Codecov.
|
alexnixon
left a comment
There was a problem hiding this comment.
LGTM, just a couple of minor comments.
| int rowCount = 0; | ||
| int columnCount = 0; | ||
| Map<Integer, SQLServerDataColumn> columnMetadata = null; | ||
| Set<String> columnList = null; |
There was a problem hiding this comment.
I think naming this columNames would be clearer
| * when a duplicate column exists | ||
| */ | ||
| private void checkDuplicateColumnName(String columnName) throws SQLServerException { | ||
| if (null != columnList) { |
There was a problem hiding this comment.
I'd rather an NPE than silently fail to check dupes if someone messes up null handling.
Better IMO would be defining the local variable like Set<String> columnNames = new HashSet<>(); and omitting this check, but I don't know if that'd violate the project's coding style.
|
@alexnixon Thanks for the review. I added the same logic for TVP since it was also applicable there, and made minor changes according to your suggestions. |
|
let's also add a test for checking duplicate column |
…nd adding a test case for this
Addresses issue #497.