655 Fix to return correct column type for spatial datatypes#657
655 Fix to return correct column type for spatial datatypes#657peterbae merged 5 commits intomicrosoft:devfrom
Conversation
…d return their own type code, not VARBYTE. also removing unused imports.
|
this appveyor failure is unrelated to this PR, it's a bug I found during internal testing that occurs only with SQL Server 2008. I will merge this PR after that issue has been solved. |
|
Actually I'm going to use a workaround for this problem, it seems like it could take a while to solve. |
Codecov Report
@@ Coverage Diff @@
## dev #657 +/- ##
============================================
- Coverage 48.12% 48.07% -0.06%
+ Complexity 2578 2576 -2
============================================
Files 113 113
Lines 26574 26579 +5
Branches 4429 4432 +3
============================================
- Hits 12790 12778 -12
- Misses 11660 11671 +11
- Partials 2124 2130 +6
Continue to review full report at Codecov.
|
| jdbcType = JDBCType.SQL_VARIANT; | ||
| } | ||
| if (SSType.UDT == typeInfo.getSSType()) { | ||
| if (typeInfo.getSSTypeName().equalsIgnoreCase("geometry")) { |
There was a problem hiding this comment.
I don't really have a problem with this, but why are we doing type check in 3 different ways. VARIANT is being checked separately outside the switch, and now GEOMETRY/GEOGRAPHY will be checked by name. Can we maybe stick to one form? This Microsoft overview of spatial datatypes seems to imply that it's only available in SQL Server 2012 and later, so shouldn't this be in the isKatamaiOrLater() block?
There was a problem hiding this comment.
We can't stick to one form. If you debug this part of the code, you'll see that the getSSType will give SQL_VARIANT or UDT. If it's UDT, we need to look at the SSTypeName to find out if it's Geometry or Geography, so this is the only way to check for these datatypes.
The overview page mentions that new spatial features have been introduced in SQL Server 2012, but this does not mean that spatial datatypes are supported starting from 2012 - it's supported starting from 2008. so we need this check for 2008 and up.
There was a problem hiding this comment.
Please create Constants for these strings and reference them here.
| jdbcType = JDBCType.SQL_VARIANT; | ||
| } | ||
| if (SSType.UDT == typeInfo.getSSType()) { | ||
| if (typeInfo.getSSTypeName().equalsIgnoreCase("geometry")) { |
There was a problem hiding this comment.
Please create Constants for these strings and reference them here.
4 participants
Fixes issue #655. Also added test cases for it.
Also removed an unused import from Geometry/Geography classes.