prowdsponsor / esqueleto

Bare bones, type-safe EDSL for SQL queries on persistent backends.
http://hackage.haskell.org/package/esqueleto
BSD 3-Clause "New" or "Revised" License
177 stars 51 forks source link

invalid sqlite sql from subquery in select with aggregate function #123

Closed vandenoever closed 8 years ago

vandenoever commented 8 years ago

Esqueleto can give invalid sql at runtime by combining a subquery with an aggregate function. The resulting SQL contains for example SELECT AVG((COUNT(...)) which gives the error "misuse of aggregate function COUNT())"

scoreQuery :: SqlExpr (Value TestId) -> SqlExpr (Value SoftwareOSId) -> SqlQuery (SqlExpr (Value Double))
scoreQuery test software = from $ \testresult -> do
    where_ $ testresult ^. TestResultTest ==. test
    where_ $ testresult ^. TestResultSoftware ==. software
    groupBy (testresult ^. TestResultTest, testresult ^. TestResultSoftware)
    let rowCount = countRows
        -- count the tests that passed
        passed = count ( case_ [ when_ (
                                testresult ^. TestResultPass ==. val True)
                            then_ (val $ Just True)]
                          (else_ $ val Nothing))
    return $ passed /. val 1.0 *. rowCount
getSoftwareOSScores :: MonadIO m => TestSetId -> ReaderT SqlBackend m [(SoftwareOSId,Double)]
getSoftwareOSScores testsetid = do
    rows :: [(Value SoftwareOSId,Value (Maybe Double))] <- select $ from $ \(testsetmember, software) -> do
        let sid = software ^. SoftwareOSId
            test = testsetmember ^. TestSetMemberTest
        score <- scoreQuery test sid
        where_ (testsetmember ^. TestSetMemberSet ==. val testsetid)
        groupBy sid
        return (sid, avg_ score)
    return $ map (\(Value software,Value score) -> (software, fromJust score)) rows

uncaught exception: SqliteException (SQLite3 returned ErrorError while attempting to perform prepare "SELECT \"software_o_s\".\"id\", AVG((COUNT(CASE WHEN (\"test_result\".\"pass\" = ?) THEN ? ELSE ? END) / ?) * COUNT(*))\nFROM \"test_set_member\", \"software_o_s\", \"test_result\"\nWHERE ((\"test_result\".\"test\" = \"test_set_member\".\"test\") AND (\"test_result\".\"software\" = \"software_o_s\".\"id\")) AND (\"test_set_member\".\"set\" = ?)\nGROUP BY \"test_result\".\"test\", \"test_result\".\"software\", \"software_o_s\".\"id\"\n": misuse of aggregate function COUNT())

SELECT "software_o_s"."id", AVG((COUNT(CASE WHEN ("test_result"."pass" = ?) THEN ? ELSE ? END) / ?) * COUNT(*))
FROM "test_set_member", "software_o_s", "test_result"
WHERE (("test_result"."test" = "test_set_member"."test") AND ("test_result"."software" = "software_o_s"."id")) AND ("test_set_member"."set" = ?)
GROUP BY "test_result"."test", "test_result"."software", "software_o_s"."id"
meteficha commented 8 years ago

This looks out of esqueleto's scope. While we strive to make errors pop at compile time, these nuances are too difficult to bother.

If you think that the generated SQL is wrong, please reopen this issue and write which (valid) SQL query should have been output.

vandenoever commented 8 years ago

The query that I am aiming for is this one:

SELECT sub.software, AVG(sub.score) FROM (
        SELECT test_result.test, test_result.software, ((COUNT(CASE WHEN (test_result.pass = 1) THEN 1 ELSE null END) * 1.0) / COUNT(*)) as score
        FROM test_result
        GROUP BY test_result.test, test_result.software
    ) as sub
GROUP BY sub.test

Perhaps I should move the where_ statements from scoreQuery to getSoftwareOSScores to achieve that.

meteficha commented 8 years ago

Hmmm, I see. Esqueleto currently doesn't support FROM queries like that, it's very difficult to support them on our current design.

vandenoever commented 8 years ago

I can imagine. You need to generate names for the subqueries to do that. Good to know the current limitation.

meteficha commented 8 years ago

It's actually more due to the type system. Take a look at how from is implemented. I guess it is possible to support these queries, but there has never been enough pressure to actually do the very hard work of bolting them in.

vandenoever commented 8 years ago

Ah, is it because you now use types defined by Persistent and subqueries would require creating types for the new 'tables' on the fly? So perhaps defining table views would be easier.

felipexpert commented 8 years ago

So, it they are not possible in Esqueleto nowadays, what would be a good workaround?