Fix non-exhaustive patterns in 'unsafeSqlAggregateFunction' #238
Conversation
| -- Since: 3.4.0.2 | ||
| valueToRawSqlParens :: SqlExpr (Value a) -> IdentInfo -> (TLB.Builder, [PersistValue]) | ||
| valueToRawSqlParens (ERaw p f) = (first (parensM p)) . f | ||
| valueToRawSqlParens (ECompositeKey _) = throw (CompositeKeyErr SqlCaseError) |
There was a problem hiding this comment.
Just noticed that the error type thrown here is different. Any suggestions?
There was a problem hiding this comment.
It seems risky to have this same function repeated and specialized more than once in the codebase. I'll go with making the error an argument here.
There was a problem hiding this comment.
I really don't like how error handling is done in esqueleto at the moment. Since this is a moving of code, it's probably fine to leave as-is. Ideally we'd just fix the representation so this can't happen, but it's not clear to me the best way to do that 🤔
| @@ -91,7 +91,7 @@ unsafeSqlAggregateFunction name mode args orderByClauses = ERaw Never $ \info -> | |||
| [] -> "" | |||
| (_:_) -> " " | |||
| (argsTLB, argsVals) = | |||
| uncommas' $ map (\(ERaw _ f) -> f info) $ toArgList args | |||
There was a problem hiding this comment.
Ugh, we even have the right warning on. I need to do a warnings-clean PR on the project soon.
|
This actually shouldn't be a problem in my work to unify the representation of SqlExpr |
|
I think this is good to merge, but CI isn't helping me :p |
|
@parsonsmatt This is fine. We should push 3.4.1.0 to hackage. |
|
looks like this changed the order of JSON objects in the postgresql test suite. @arthurxavierx @parsonsmatt i dont particularly care to fix this as i think 3.5.0 essentially reverts this change. |
|
Thank you @belevy! :) |
Got the following error when using
unsafeSqlAggregateFunction:This PR fixes the problem.
Before submitting your PR, check that you've:
@sincedeclarations to the Haddock.stylish-haskelland otherwise adhered to the style guide.After submitting your PR: