Fix for #50 #53
Fix for #50 #53
Conversation
There was a problem hiding this comment.
I don't think it's correct to just claim v2 support, without actually adding v2 support. See https://github.com/dotMorten/Microsoft.SqlServer.Types/blob/main/src/Microsoft.SqlServer.Types/ShapeData.cs#L236-L239
Also there's a huge amount of white space/newline changes in the PR
| @@ -98,6 +98,9 @@ internal struct Figure | |||
| /// </summary> | |||
| internal struct ShapeData | |||
| { | |||
| // As per https://docs.microsoft.com/en-us/openspecs/sql_server_protocols/ms-ssclrt/77460aa9-8c2f-4449-a65e-1d649ebd77fa | |||
| public const byte MAX_GEOGRAPHY_SERIALIZATION_FORMAT_SUPPORTED = 2; | |||
There was a problem hiding this comment.
That's not really the case though is it? The library currently does not support the curve segments that were added to v2
There was a problem hiding this comment.
Mmmm..the point that I have created (on Azure SQL) a POLYGON and still got a binary format that shows 2 as version, even if I'm not using curve segments. Without this hack/fix, the geography will not be handled correctly
There was a problem hiding this comment.
I don't know why it's serialized as v2 when not needed, but this library currently doesn't support v2, so it's correctly failing until #9 has been addressed.
I'm a little worried what this means to just ignore it here, without at least handling the extra fields and skipping correctly over and failing if a curved geometry is hit
|
Closing this, since I'd rather we get proper v2 curve support before trying to read it |
Seems that support to Geography binary format v2 is already there, but when reading the byte stream, the maximum expected version was still set to 1