Added basic JSON column support #65
Conversation
|
Changed it to serialize/deserialize a |
|
I didn't want it to get confused with the JSON class. But can change if
you'd like
…On Fri, Aug 17, 2018 at 7:05 PM Julien Portalier ***@***.***> wrote:
***@***.**** commented on this pull request.
Looks good. Maybe just use MySQL::Type::JSON instead of MySQL::Type::Json
but that's nitpicking.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#65 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFXk7BQoAVvicYkAcGEBI9ZEfw7AHIHFks5uR0wcgaJpZM4V8-yb>
.
|
|
@bcardiff any input? |
There was a problem hiding this comment.
Besides some specific feedback, this is my overall thinking of the topic:
-
I would prefer that the JSON parsing would work directly from the IO instead of reading all the input with
read_lenenc_string. For this aIO::Sizedshould probably be used, plus some checks in the connection. (I would even want to expose that when doingResultSet#read(IO)). -
I would like see support for types using
JSON.mappingin order to have a type safe type version. Both for reading and writing. Similar to 1, without allocating the whole json in a string before sending it to the socket. -
At the same time JSON api in stdlib had had some changes in recent versions. I would wait for them to settle down a bit before adding them as a hard dependency.
I would move forward using IO for JSON types for now. At build later the parsing of json on top of that, or at least allow the user to choose how to do it.
| @@ -49,6 +49,7 @@ DB::DriverSpecs(MySql::Any).run do | |||
| sample_value Time.utc(2016, 2, 15, 10, 15, 30, nanosecond: 543_012_000), "timestamp(6)", "TIMESTAMP '2016-02-15 10:15:30.543012'" | |||
| sample_value Time::Span.new(0, 10, 15, 30, nanoseconds: 543_000_000), "Time(3)", "TIME '10:15:30.543'" | |||
| sample_value Time::Span.new(0, 10, 15, 30, nanoseconds: 543_012_000), "Time(6)", "TIME '10:15:30.543012'" | |||
| sample_value JSON::Any.new({"example" => JSON::Any.new("json")}), "JSON", "'{\"example\": \"json\"}'", type_safe_value: false | |||
There was a problem hiding this comment.
I thought it would be a type_safe_value why it's not?
| str = packet.read_lenenc_string.lchop("\u0013") | ||
| JSON.parse(str) | ||
| rescue e | ||
| nil |
There was a problem hiding this comment.
Why exceptions should be swallowed?
| end | ||
|
|
||
| def self.read(packet) | ||
| str = packet.read_lenenc_string.lchop("\u0013") |
There was a problem hiding this comment.
Does MySQL sends trailing \u0013 on all json values? Is there a documentation/reference for this?
There was a problem hiding this comment.
it seems to be a prefix for all the JSON data. I have not been able to find mysql documentation on it however.
|
Brining branch up to speed and checking in. Does this need more work? |
At a minimum we should be able to read/write to JSON columns via a String. This solves for that.
#64
amberframework/granite#193