vapor / fluent-kit

Swift ORM (queries, models, and relations) for NoSQL and SQL databases
MIT License
213 stars 116 forks source link

QueryBuilder.sum(KeyPath) always returns nil #379

Open Christian-Seiler opened 4 years ago

Christian-Seiler commented 4 years ago

I have a Class Course and try to use the sum method in the following way:

final class Course: Model, Content {
   static let schema: String  = "courses"
   ...
   @Field(key: "credits")
   var credits: Int
    ...
}

Course.query(on: database)
    .filter(...)
    .sum(\.$credits)

As outlined in the documentation I would expect the query to return type Int.

All aggregate methods except count return the field's value type as a result.

However, I receive an Optional instead. Furthermore, the result of it always is nil. When I query with the code below, I receive the correct value.

Course.query(on: database)
    .filter(...)
    .all()
    .map { (courses) -> Int in
          var total = 0
          courses.forEach { total += $0.credits }
          return total
    }
Craz1k0ek commented 4 years ago

I tried reproducing your issue, but it appears to work fine for me. The model and migration:

import Fluent
import Vapor

final class Course: Model, Content {
    static let schema: String  = "courses"

    @ID(key: .id)
    var id: UUID?

    @Field(key: "credits")
    var credits: Int
}

struct CourseMigration: Migration {
    func prepare(on database: Database) -> EventLoopFuture<Void> {
        database.schema(Course.schema)
            .id()
            .field("credits", .int)
            .create()
    }

    func revert(on database: Database) -> EventLoopFuture<Void> {
        database.schema(Course.schema).delete()
    }
}

I then proceeded to apply the migration in configuration.swift

app.databases.use(.sqlite(.file("db.sqlite")), as: .sqlite)
app.migrations.add(CourseMigration())

And for the routes.swift

func routes(_ app: Application) throws {
    app.get { req in
        return Course.query(on: req.db).filter(\.$credits < 100).sum(\.$credits) ?? -1
    }

    app.get("hello") { req -> EventLoopFuture<String> in
        let c = Course()
        c.credits = Int.random(in: 0 ..< 500)
        return c.save(on: req.db).transform(to: "OK")
    }
}

Are you sure you ran the migrations with vapor run migrate before issuing vapor run? What version of Vapor are you running? What database type do you use? If you copy and paste this code, does this problem still affect you?

Christian-Seiler commented 4 years ago

I use Postgres Database. Since my code with .all().map() returns the correct Int value, this is not an issue with Migration. With the same Database / Data, sum() as well as .all().map() must return the same values, but sum() always returns nil!

I have the following versions after swift package update:

vapor 4.29.3
fluent 4.0.0
fluent-postgres-driver 2.1.0
Craz1k0ek commented 4 years ago

I've managed to reproduce your issue. It is, afaik, related to postgres and its built in sum function. When executing the following SQL, it states that the data type is bigint, resulting in the issue.

SELECT column_name, data_type FROM information_schema.columns WHERE table_name = 'courses';
#  column_name | data_type 
# -------------+-----------
#  id          | uuid
#  credits     | bigint

And to get the type of the returned sum value we can do the following.

SELECT pg_typeof(sum) FROM (SELECT SUM(credits) from courses) as NESTED;
# pg_typeof 
# -----------
# numeric

The numeric result type is interpreted as Double when in Swift. This all traces back to PostgresRow+Database.swift located in the fluent-postgres-driver package at line 42. I executed some lldb commands to find out what the issue is.

(lldb) po try self.row.sql(decoder: self.decoder).decode(column: self.columnName(key), as: Int?.self)
nil

(lldb) po try self.row.sql(decoder: self.decoder).decode(column: self.columnName(key), as: Double?.self)
▿ Optional<Double>
  - some : 4009.0

And you can now see that it translates perfectly fine to a Double. I'm not quite sure what the underlying fix is, but I'll create a bug report in the fluent-postgres-driver package and reference this one.

Postgress aggregated functions