crystal-lang / crystal-mysql

MySQL connector for Crystal
MIT License
107 stars 36 forks source link

Added JSON Type #24

Open aurimasniekis opened 7 years ago

aurimasniekis commented 7 years ago

I have tried to implement native json type. With this code it works for reading and parsing. But I don't know if it's correct way to implement it.

Especially with this part:

def self.type_for(t : JsonType.class)
    MySql::Type::JSON
end
bcardiff commented 7 years ago

ResultSet#read should receive the class of object that expected to be returned. In this case I would expect that rs.read(JSON::Any) will return the next value as a JSON::Any.

Since JSON::Any is a union there might be some issue, but at least from the following snippet it seems the overload are picked in correctly.

require "json"

def f(t : JSON::Any.class)
  "a json any"
end

def f(t : String.class)
  "a string"
end

pp f(JSON::Any) # => "a json any"
pp f(String)    # => "a string"

I would say to try that way first.

When writing the json object to the packet we should write directly to the IO. That might require some new methods in packet probably.

aurimasniekis commented 7 years ago

Sorry, but my current knowledge is really limited with Crystal, so I can try to help but not much more :D

spalladino commented 7 years ago

@aurimasniekis no worries, everyone started out with a really limited knowledge here :-)

I'd suggest going through how support for other types is implemented. As @bcardiff mentioned, ResultSet#read typically receives as an argument the class of the object to read, so the first step would be to add support for JSON::Any there. I'd also advise to try and become familiar with JSON parsing in Crystal, which can be quite difficult (especially if coming from a dynamic language).

The other thing Brian was mentioning was to write the JSON directly on the stream. One of the goals of all crystal-db methods (and its drivers) is to be as efficient as possible by not allocating unnecessary memory. So, instead of serializing an object to a string, and copying that string to the data stream, you could try and serialize the object directly onto the stream (note that to_json has an overload that receives an IO, which could help here).