vapor / mysql-nio

🐬 Non-blocking, event-driven Swift client for MySQL.
MIT License
87 stars 28 forks source link

Unable to Decode sum(int(11)) Values in MySQL #18

Closed thecheatah closed 4 years ago

thecheatah commented 4 years ago

I am not able to decode an integer for a query like

SELECT sum(unread_messages_count) `count` FROM `chat_participant` WHERE `user_id` = \(bind: userId)

where unread_messages_count is an int(11) in mysql. Going through the debugger, it seems that we hit the default case on line 313 of MySQLData.swift.

The value of MySQLData.type that the switch statement is switching off of has a raw value of 246. Type 246 is newdecimal from MySQLProtocol+DataType.swift line 68. Should we handle converting newdecimal to Int? The default .sum() aggregation function tries to decode to Int as well and I was having trouble with the following code:

    return ChatParticipant.query(on: db)
      .filter(\.$user.$id, .equal, userId)
      .sum(\.$numberOfUnreadMessages)
      .unwrap(or: Abort(.internalServerError, reason: "Unable to unwrap sum(numberOfUnreadMessages) for user \(userId)"))

I was getting the same decoding error as the raw query.

I can see why they might use decimal as the summed up columns might not be integers. Looking at the MySQLNIO decoding logic, it doesn't seem like we handle decoding of newdecimal anywhere. newdecimal seems to be unimplemented.

Also, does decoding sum as int by default make sense? (I can be convinced either way) This is dynamic based on the type of the field. The "issue" is that for an int column, mysql is returning a newdecimal that cannot be parsed as an int.

I am using the latest docker image for MySQL 5.7.

My work around for this bug was to cast the sum aggregate as SIGNED

SELECT CAST(sum(unread_messages_count) as SIGNED) `count` FROM `chat_participant` WHERE `user_id` = \(bind: userId)

Added test case that fails:

    func testDecodingSumOfInts() throws {
        let conn = try MySQLConnection.test(on: self.eventLoop).wait()
        defer { try! conn.close().wait() }
        let dropResults = try conn.simpleQuery("DROP TABLE IF EXISTS foos").wait()
        XCTAssertEqual(dropResults.count, 0)
        let createResults = try conn.simpleQuery("CREATE TABLE foos (`item_count` int(11))").wait()
        XCTAssertEqual(createResults.count, 0)
        let rows = try conn.simpleQuery("SELECT sum(`item_count`) as sum from foos").wait()
        guard rows.count == 1 else {
            XCTFail("invalid row count")
            return
        }
        XCTAssertNotNil(rows[0].column("sum"))
        XCTAssertEqual(rows[0].column("sum")?.string, "0")
        XCTAssertEqual(rows[0].column("sum")?.double, 0)
        XCTAssertEqual(rows[0].column("sum")?.int, 0)
    }

Debugger returns

(lldb) po rows[0].column("sum")
▿ Optional<MySQLData>
  ▿ some : nil
    ▿ type : MYSQL_TYPE_NEWDECIMAL
      - rawValue : 246
    - format : MySQLNIO.MySQLData.Format.text
    - buffer : nil
    - isUnsigned : false
thecheatah commented 4 years ago

The native D mysql driver seems to have run into a similar issue: https://github.com/mysql-d/mysql-native/issues/39

thecheatah commented 4 years ago

newdecimal appears to be a length encoded string as documented here: https://mariadb.com/kb/en/resultset-row/#decimal-binary-encoding

From observation a 0 is represented as a nil buffer. 1 is represented as the string 1. From the documentation above, numbers can also be 123.456.

thecheatah commented 4 years ago

Example encoding of 1 as a new decimal

(lldb) po self
▿ <MYSQL_TYPE_NEWDECIMAL>
  ▿ type : MYSQL_TYPE_NEWDECIMAL
    - rawValue : 246
  - format : MySQLNIO.MySQLData.Format.binary
  ▿ buffer : Optional<ByteBuffer>
    ▿ some : ByteBuffer { readerIndex: 0, writerIndex: 1, readableBytes: 1, capacity: 1, slice: _ByteBufferSlice { 7..<8 }, storage: 0x00000001018d8600 (32768 bytes) }
      ▿ _storage : <_Storage: 0x100cad320>
      - _readerIndex : 0
      - _writerIndex : 1
      ▿ _slice : _ByteBufferSlice { 7..<8 }
        - upperBound : 8
        ▿ _begin : 7
          - b12 : 7
          - b3 : 0
  - isUnsigned : false

(lldb) po buffer.readString(length: buffer.readableBytes)
▿ Optional<String>
  - some : "1"