WIP to add support for Geometry (POINT) type #197
Conversation
| func convertToData() -> Data { | ||
| switch self { | ||
| case .point(let x, let y): | ||
| let bufferSize = MySQLGeometry.headerSize + 2 * MemoryLayout<Double>.size |
There was a problem hiding this comment.
The MySQL spec requires these to be 8 bytes. I don't know about in Swift, but in C and C++ a double isn't guaranteed to be 8 bytes. Just to have higher precision than a float.
WKB uses 1-byte unsigned integers, 4-byte unsigned integers, and 8-byte double-precision numbers (IEEE 754 format). A byte is eight bits.
There was a problem hiding this comment.
In C we'd typically use a static assert (compile time) to ensure that the double is the size we're expecting or update the algorithm to allow for a double to be another size
There was a problem hiding this comment.
As far as I can tell it is 8 bytes. The rest of the MySQL code assumes this as well. I think I will change it to a literal "16" with a comment though.
| } | ||
|
|
||
| let _: UInt32 = try buffer.requireInteger() | ||
| let _: UInt8 = try buffer.requireInteger() |
There was a problem hiding this comment.
I don't know if it's possible, but you may want to assert that this is little-endian.
|
|
||
| extension MySQLGeometry: ReflectionDecodable { | ||
| public static func reflectDecoded() throws -> (MySQLGeometry, MySQLGeometry) { | ||
| return (.point(x: 0, y: 0), .point(x: 0, y: 1)) |
There was a problem hiding this comment.
Ah, the wonderful world of ReflectionDecodable, see https://github.com/vapor/core/blob/master/Sources/Core/CodableReflection/ReflectionDecodable.swift. However, it seems I don't actually need this one anymore. I believe I needed it earlier to exclude a property on my model with this type, the process of which required reflection.
|
I cannot work on this before august 6 the earliest so if anyone else has time to pick up the unit tests, that would be awesome!
… Den 18. jul. 2018 kl. 07.58 skrev Tanner ***@***.***>:
@tanner0101 commented on this pull request.
Nice, this LGTM. Just needs some unit tests. :)
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
siemensikkema
force-pushed
the
feature/geometry-support
branch
from
58ad484 to
499a317
Compare
|
Moving to vapor/mysql-nio#42 |
Points can now be correctly stored and retrieved.