crystal-lang / crystal-mysql

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

support TINYINT(1) #12

Closed rdp closed 7 years ago

rdp commented 7 years ago

I think that's what generates this anyway:

not supported (Exception) [4387954258] CallStack::unwind:Array(Pointer(Void)) +82 [4387954161] CallStack#initialize:Array(Pointer(Void)) +17 [4387954120] CallStack::new:CallStack +40 [4387860777] raise:NoReturn +25 [4387860737] raise:NoReturn +17 [4389068958] MySql::Type+@MySql::Type::read:NoReturn +30 [4389072710] *MySql::ResultSet#read:(Float32 | Float64 | Int32 | Int64 | Slice(UInt8) | String | Time | Nil) +1238

(the current error message doesn't say "what" isn't supported, as it were).

crisward commented 7 years ago

It'd may be nice to treat TINYINT(1) as a bool... especially for json output. Thoughts?

crisward commented 7 years ago

I've added tiny int to mysql driver. As mysql doesn't support Bool type I've created this converter for json. The pull request is still outstanding, my fork is here - https://github.com/crisward/crystal-mysql

 class Int8ToBool
    def to_json(value : Int8, io : IO)
      val = value > 0 ? true : false
      io << val
    end

    def from_json(value : JSON::PullParser) : Int8
      val = value.read_bool
      val == true ? 1_i8 : 0_i8
    end
  end

You can use it in your json mapping with

ie

    JSON.mapping({
      somefield:     {type: Int8, nilable: true, converter: Int8ToBool.new},
    })
asterite commented 7 years ago

Yes, I think tinyint should be a Bool, not Int8

crisward commented 7 years ago

The mysql driver returns an int8, a tinyint can store 0-256, so does have other uses. (http://dev.mysql.com/doc/internals/en/binary-protocol-value.html#packet-ProtocolBinary::MYSQL_TYPE_TINY)

I think leaving it as an int8 and converting to a bool in the json layer seems to make most sense. I personally only use tiny int for bool's but I can imagine other uses.

bcardiff commented 7 years ago

15 was merged.

As @crisward says tinyint is always 1 byte so it won't be right to treat them as booleans. Int8ToBool converter it the way to go.