-
Notifications
You must be signed in to change notification settings - Fork 3
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
Add sqlWhereAnyE #16
Add sqlWhereAnyE #16
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM modulo comments.
let sql = sqlForWhereAny l | ||
sqlWhereE (DBBaseLineConditionIsFalse sql) sql | ||
|
||
sqlWhereAnyE :: (DBExtraException e, MonadState v m, SqlWhere v) => e -> [State SqlAll ()] -> m () |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm aware that most top-level functions in this file lack Haddock comments, but would be nice if we started adding them to the new ones at least.
sqlWhereAny l = sqlWhere $ "(" <+> smintercalate "OR" (map (parenthesize . toSQLCommand . flip execState (SqlAll [])) l) <+> ")" | ||
sqlWhereAny l = do | ||
let sql = sqlForWhereAny l | ||
sqlWhereE (DBBaseLineConditionIsFalse sql) sql |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't you just sqlWhere
here instead of inlining it?
sqlWhereAnyE :: (DBExtraException e, MonadState v m, SqlWhere v) => e -> [State SqlAll ()] -> m () | ||
sqlWhereAnyE e = sqlWhereE e . sqlForWhereAny | ||
|
||
sqlForWhereAny :: [State SqlAll ()] -> SQL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sqlFor
naming convention is not used anywhere else in this file, IMO something like sqlWhereAnyImpl
would be more self-explanatory.
sqlWhereAnyE :: (DBExtraException e, MonadState v m, SqlWhere v) => e -> [State SqlAll ()] -> m () | ||
sqlWhereAnyE e = sqlWhereE e . sqlForWhereAny | ||
|
||
sqlForWhereAny :: [State SqlAll ()] -> SQL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[State SqlAll ()]
From looking at the code, it seems that it would make sense to add a SqlAny
newtype as well, though that's a bigger refactoring (and therefore not required for this PR to be merged).
Travis failure is due to the released |
Made a Hackage revision for hpqtypes-1.6.0.0, this should made the build green. |
It's green, merging. Will do a minor release later today. |
Released |
No description provided.