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.

Sign up for GitHub

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 limitMaybe_, offsetMaybe_ + tests #633

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+57 −3
Open

Add limitMaybe_, offsetMaybe_ + tests #633

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9d13a69
Add limitMaybe_, offsetMaybe_ + tests
thomasjm Feb 18, 2022
dc6d15e
Formatting
thomasjm Feb 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 22 additions & 2 deletions beam-core/Database/Beam/Query/Combinators.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ module Database.Beam.Query.Combinators
, QIfCond, QIfElse
, (<|>.)

, limit_, offset_
, limit_, limitMaybe_
, offset_, offsetMaybe_

, as_

Expand All @@ -66,8 +67,8 @@ module Database.Beam.Query.Combinators
, orderBy_, asc_, desc_, nullsFirst_, nullsLast_
) where

import Database.Beam.Backend.Types
import Database.Beam.Backend.SQL
import Database.Beam.Backend.Types

import Database.Beam.Query.Internal
import Database.Beam.Query.Ord
Expand All @@ -83,6 +84,7 @@ import Control.Applicative
import Data.Maybe
import Data.Proxy
import Data.Time (LocalTime)
import Unsafe.Coerce (unsafeCoerce)

import GHC.TypeLits (TypeError, ErrorMessage(Text))

Expand Down Expand Up @@ -335,6 +337,15 @@ limit_ :: forall s a be db
limit_ limit' (Q q) =
Q (liftF (QLimit limit' q (rewriteThread (Proxy @s))))

-- | Conditionally limit the number of results returned by a query.
limitMaybe_ :: forall s a be db
. ( Projectible be a
, ThreadRewritable (QNested s) a )
=> Maybe Integer -> Q be db (QNested s) a -> Q be db s (WithRewrittenThread (QNested s) s a)
limitMaybe_ (Just limit') (Q q) =
Q (liftF (QLimit limit' q (rewriteThread (Proxy @s))))
limitMaybe_ Nothing (Q q) = Q (unsafeCoerce q)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should be able to use rewriteThread

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, could you give a hint on how that would look? I haven't grokked this stuff yet...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tathougies I'm going through old PRs in and was blocked on this question -- could you clarify?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, so the type doesn't match I believe because (QNested s) doesnt unify with WithRewrittenThread (QNested s) s. The latter unwraps one level of QNested`, which is what the rewriteThread function is fro. I believe if you do.

rewriteThread (Proxy @s) q instead of the unsafeCoerce q , that should work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried some stuff like that and wasn't able to get it to work:

Building library for beam-core-0.9.2.1..
[28 of 32] Compiling Database.Beam.Query.Combinators

/home/tom/tools/beam/beam-core/Database/Beam/Query/Combinators.hs:347:32: error:
    • Couldn't match type: WithRewrittenThread
                             s0 s (QM be db (QNested s) a)
                     with: Control.Monad.Free.Church.F
                             (QF be db s) (WithRewrittenThread (QNested s) s a)
      Expected: QM be db s (WithRewrittenThread (QNested s) s a)
        Actual: WithRewrittenThread s0 s (QM be db (QNested s) a)
      The type variable ‘s0’ is ambiguous
    • In the first argument of ‘Q’, namely
        ‘(rewriteThread (Proxy @s) q)’
      In the expression: Q (rewriteThread (Proxy @s) q)
      In an equation for ‘limitMaybe_’:
          limitMaybe_ Nothing (Q q) = Q (rewriteThread (Proxy @s) q)
    • Relevant bindings include
        q :: QM be db (QNested s) a
          (bound at Database/Beam/Query/Combinators.hs:347:24)
        limitMaybe_ :: Maybe Integer
                       -> Q be db (QNested s) a
                       -> Q be db s (WithRewrittenThread (QNested s) s a)
          (bound at Database/Beam/Query/Combinators.hs:345:1)
    |
347 | limitMaybe_ Nothing (Q q) = Q (rewriteThread (Proxy @s) q)
    |                                ^^^^^^^^^^^^^^^^^^^^^^^^^^

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tathougies pinging again, would love to get this merged

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tathougies another ping, please help! or maybe we could just merge as-is?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tathougies here is the ping for 2024 :)


-- | Drop the first `offset'` results.
offset_ :: forall s a be db
. ( Projectible be a
Expand All @@ -343,6 +354,15 @@ offset_ :: forall s a be db
offset_ offset' (Q q) =
Q (liftF (QOffset offset' q (rewriteThread (Proxy @s))))

-- | Conditionally drop the first `offset'` results.
offsetMaybe_ :: forall s a be db
. ( Projectible be a
, ThreadRewritable (QNested s) a )
=> Maybe Integer -> Q be db (QNested s) a -> Q be db s (WithRewrittenThread (QNested s) s a)
offsetMaybe_ (Just offset') (Q q) =
Q (liftF (QOffset offset' q (rewriteThread (Proxy @s))))
offsetMaybe_ Nothing (Q q) = Q (unsafeCoerce q)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.


-- | Use the SQL @EXISTS@ operator to determine if the given query returns any results
exists_ :: ( BeamSqlBackend be, HasQBuilder be, Projectible be a)
=> Q be db s a -> QExpr be s Bool
Expand Down
36 changes: 35 additions & 1 deletion beam-core/test/Database/Beam/Test/SQL.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1032,7 +1032,9 @@ selectCombinators =
limitOffset :: TestTree
limitOffset =
testGroup "LIMIT/OFFSET support"
[ limitSupport, offsetSupport, limitOffsetSupport
[ limitSupport, maybeLimitSupportJust, maybeLimitSupportNothing
, offsetSupport, maybeOffsetSupportJust, maybeOffsetSupportNothing
, limitOffsetSupport

, limitPlacedOnUnion ]
where
Expand All @@ -1044,6 +1046,22 @@ limitOffset =
selectLimit @?= Just 20
selectOffset @?= Nothing

maybeLimitSupportJust =
testCase "Maybe LIMIT support (Just)" $
do SqlSelect Select { selectLimit, selectOffset } <-
pure $ selectMock $ limitMaybe_ (Just 20) (all_ (_employees employeeDbSettings))

selectLimit @?= Just 20
selectOffset @?= Nothing

maybeLimitSupportNothing =
testCase "Maybe LIMIT support (Nothing)" $
do SqlSelect Select { selectLimit, selectOffset } <-
pure $ selectMock $ limitMaybe_ Nothing (all_ (_employees employeeDbSettings))

selectLimit @?= Nothing
selectOffset @?= Nothing

offsetSupport =
testCase "Basic OFFSET support" $
do SqlSelect Select { selectLimit, selectOffset } <-
Expand All @@ -1052,6 +1070,22 @@ limitOffset =
selectLimit @?= Nothing
selectOffset @?= Just 102

maybeOffsetSupportJust =
testCase "Maybe OFFSET support (Just)" $
do SqlSelect Select { selectLimit, selectOffset } <-
pure $ selectMock $ offsetMaybe_ (Just 2) $ offset_ 100 (all_ (_employees employeeDbSettings))

selectLimit @?= Nothing
selectOffset @?= Just 102

maybeOffsetSupportNothing =
testCase "Maybe OFFSET support (Nothing)" $
do SqlSelect Select { selectLimit, selectOffset } <-
pure $ selectMock $ offsetMaybe_ Nothing $ offset_ 100 (all_ (_employees employeeDbSettings))

selectLimit @?= Nothing
selectOffset @?= Just 100

limitOffsetSupport =
testCase "Basic LIMIT .. OFFSET .. support" $
do SqlSelect Select { selectLimit, selectOffset } <-
Expand Down