Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Variadic groupBy / orderBy #30

Open
grosch opened this issue Aug 7, 2018 · 5 comments
Open

Variadic groupBy / orderBy #30

grosch opened this issue Aug 7, 2018 · 5 comments
Labels
enhancement New feature or request
Projects

Comments

@grosch
Copy link
Contributor

grosch commented Aug 7, 2018

@tanner0101 , after out long chat on Discord, I ended up with these convenience methods. Like you said, they only work if all arguments are the same for the groupBy and orderBy, but that you knew how to fix. Hopefully this can turn into a real thing

extension SQLSelectBuilder {
    // This one needs to REPLACE the existing one as it adds the column
    public func groupBy<T,V>(_ keyPaths: KeyPath<T, V>...) -> Self
        where T: SQLTable
    {
        return keyPaths.map { keyPath in
            return groupBy(.column(.keyPath(keyPath))).column(expression: .column(.keyPath(keyPath)))
        }.first!
    }

    public func sum<T,V>(_ keyPath: KeyPath<T, V>, as alias: Connection.Query.Select.SelectExpression.Identifier? = nil) -> Self where T: SQLTable {
        return column(function: "SUM", .expression(.column(.keyPath(keyPath))), as: alias)
    }

    public func count<T,V>(_ keyPath: KeyPath<T, V>, as alias: Connection.Query.Select.SelectExpression.Identifier? = nil) -> Self where T: SQLTable {
        return column(function: "COUNT", .expression(.column(.keyPath(keyPath))), as: alias)
    }

    public func orderBy<T,V>(_ keyPaths: KeyPath<T,V>..., direction: Connection.Query.Select.OrderBy.Direction = .ascending) -> Self  where T: SQLTable {
        keyPaths.forEach { keyPath in
            select.orderBy.append(.orderBy(.column(.keyPath(keyPath)), direction))
        }

        return self
    }
}
@MrMage MrMage changed the title Productionalize these Productionalize these aggregate functions Aug 8, 2018
@tanner0101 tanner0101 added the enhancement New feature or request label Sep 20, 2018
@tanner0101
Copy link
Member

tanner0101 commented Sep 20, 2018

I've added the count and sum methods as static conveniences to SQLSelectStatement in #35. This will allow for you to do something like:

try conn.select()
    .column(.count(\Planet.name))
    .from(Planet.self)
    .where(\Planet.galaxyID, .equal, 5)
    .run().wait()

try conn.select()
    .column(.count(.all))
    .from(Planet.self)
    .where(\Planet.galaxyID, .equal, 5)
    .run().wait()

try conn.select()
    .column(.sum(\Planet.id), as: "id_sum")
    .from(Planet.self)
    .where(\Planet.galaxyID, .equal, 5)
    .run().wait()

I think this is a bit more future proof than adding directly to the SQLSelectBuilder as the amount of functions there may start to become quite bloated.

I'll leave this issue open to discuss the best way of adding orderBy and groupBy. Could you reiterate here what the use case for those variadic methods is?

@tanner0101 tanner0101 changed the title Productionalize these aggregate functions variadic groupBy / orderBy Sep 20, 2018
@grosch
Copy link
Contributor Author

grosch commented Sep 20, 2018

The variadics are just a convenience. I usually am doing more than one SUM, for example, so it just makes the code a bit cleaner. The group by one, though, automatically adds the item being grouped to the select list. If you're grouping by it, you MUST select it, so why make them explicitly add it?

@grosch
Copy link
Contributor Author

grosch commented Nov 5, 2018

Hey @tanner0101, any update on the groupBy option?

@tanner0101
Copy link
Member

@grosch I'm not sure the signature as exists would work:

func groupBy<T,V>(_ keyPaths: KeyPath<T, V>...)

This only allows you to add key paths of the same model and value. For example, what if you wanted to do something like:

try conn.select()
    .column(.star)
    .from(Planet.self)
    .join(Galaxy.self, on: \Planet.galaxyID == \Galaxy.id)
    .groupBy(\Planet.name, \Galaxy.name)
    .run().wait()

This wouldn't work because the root model type is not equal (Galaxy != Planet). The same thing also prevents grouping by two different columns on the same model if the column type is different.

@grosch
Copy link
Contributor Author

grosch commented Nov 16, 2018

@tanner0101 The sql you showed isn't valid though. If you GROUP BY something, those items all have to be in the select list.

@tanner0101 tanner0101 added this to To Do in Vapor 4 via automation Apr 22, 2020
@tanner0101 tanner0101 moved this from To Do to Backlog in Vapor 4 Apr 22, 2020
@tanner0101 tanner0101 changed the title variadic groupBy / orderBy Variadic groupBy / orderBy Apr 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Vapor 4
  
Backlog
Development

No branches or pull requests

2 participants