Add `selectOne` #265

ibarrae · 2021-06-02T13:58:11Z

Closes #257

Before submitting your PR, check that you've:

Bumped the version number.
Documented new APIs with Haddock markup.
Added @since declarations to the Haddock.
Ran stylish-haskell and otherwise adhered to the style guide.

After submitting your PR:

Update the Changelog.md file with a link to your PR.
Check that CI passes (or if it fails, for reasons unrelated to your change, like CI timeouts).

belevy · 2021-06-02T14:04:33Z

I'd prefer that the function be called selectFirst, unfortunately that would technically be a major API break since we're re-exporting persistent. Perhaps we could put this in the release that removes the old syntax?

ibarrae · 2021-06-02T14:07:01Z

I'd prefer that the function be called selectFirst, unfortunately that would technically be a major API break since we're re-exporting persistent. Perhaps we could put this in the release that removes the old syntax?

Thanks for the quick reply @belevy :) I was wondering about that, to avoid conflicts with existent persistent function, thus I thought as a workaround (for now) to get it called in a different way.

And correct me if I'm wrong, but you are mentioning to put this as part of ~~4.0.0.0~~ 3.6 right?

…ent`

belevy · 2021-06-02T14:39:24Z

@parsonsmatt Correct me if I'm wrong but this would require a bump in the major version right i.e 4.0.0

parsonsmatt · 2021-06-02T15:15:02Z

esqueleto uses PVP versioning so we could put this out with selectFirst as an esqueleto function with 3.6.0.0

parsonsmatt

I'd rather have this out now with selectOne as the name than do a major version bump to give it the name selectFirst.

This is great to have, thanks for the PR! 😄

parsonsmatt · 2021-06-02T15:15:42Z

src/Database/Esqueleto.hs

@@ -129,7 +130,7 @@ module Database.Esqueleto {-# WARNING "This module will switch over to the Exper
  ) where

 import Database.Esqueleto.Legacy
-import Database.Esqueleto.Internal.PersistentImport
+import Database.Esqueleto.Internal.PersistentImport hiding (selectFirst)


Instead of hiding the import, we should just not re-export it from PersistentImpot

parsonsmatt · 2021-06-02T15:16:57Z

src/Database/Esqueleto/Legacy.hs

@@ -130,7 +131,7 @@ module Database.Esqueleto.Legacy
  ) where

 import Database.Esqueleto.Internal.Internal
-import Database.Esqueleto.Internal.PersistentImport
+import Database.Esqueleto.Internal.PersistentImport hiding (selectFirst)


Same issue here (and elsewhere) - let's keep this an open import and hide the export at the source.

Suggested change

import Database.Esqueleto.Internal.PersistentImport hiding (selectFirst)

import Database.Esqueleto.Internal.PersistentImport

parsonsmatt · 2021-06-02T15:19:44Z

src/Database/Esqueleto/Internal/Internal.hs

+-- @
+
+selectFirst :: (SqlSelect a r, MonadIO m) => SqlQuery a -> SqlReadT m (Maybe r)
+selectFirst query = fmap Maybe.listToMaybe $ select $ limit 1 >> query


I also like the idea of naming this function selectFirst, but the API break with persistent does unfortunately require a major version bump (to 3.6.0.0).

It's such a useful function, I'd like to expose it now. We can do this with a minor version bump (3.5.1.0), as long as it does not conflict. selectOne is another fine name for it, and I'd honestly be fine with having that name.

Sounds great to me 🎉 will rename it accordingly and remove the unnecessary hiding statements, thanks!

parsonsmatt · 2021-06-02T15:21:18Z

src/Database/Esqueleto/Internal/Internal.hs

+--  'select' $
+--      'from' $ \person -> do
+--          'limit' 1
+--          return person


Let's use experimental syntax for the new examples.

Suggested change

-- 'select' $

-- 'from' $ \person -> do

-- 'limit' 1

-- return person

-- 'select' $ do

-- person <- 'from' $ table @Person

-- 'limit' 1

-- return person

parsonsmatt · 2021-06-02T15:21:46Z

src/Database/Esqueleto/Internal/Internal.hs

+--  'selectFirst' $
+--      'from' $ \person ->
+--          return person


Same as below - let's use the Experimental syntax instead, since all code will be switching to it.

ibarrae · 2021-06-02T16:41:32Z

Thanks for all the suggestions @belevy @parsonsmatt, I updated the branch accordingly and also updated the version number. Any more feedback is more than welcome 🎉

belevy · 2021-06-15T13:48:24Z

test/Common/Test.hs

+    describe "selectOne" $ do
+        let personQuery =
+                selectOne $
+                    from $ \person -> do


could you change this test query to use the Experimental syntax please. Were in the process of deprecating the old syntax so it will eventually need to change anyway.

For sure @belevy, I still need to get used to the upcoming syntax 😄. You can take a look at those changes in this commit

parsonsmatt

looks great, thank you 😄

parsonsmatt · 2021-06-17T16:10:23Z

src/Database/Esqueleto/Internal/Internal.hs

+--  'select' $ do
+--      person <- 'from' $ 'table' @Person
+--      'limit' 1
+--      return person


fantastic 😄

parsonsmatt · 2021-06-17T16:10:50Z

test/Common/Test.hs

+        itDb "returns Nothing" $ do
+            res <- personQuery
+            asserting $
+                res `shouldBe` (Nothing :: Maybe (Value PersonId))


parsonsmatt · 2021-06-17T19:18:16Z

released! thanks 😄

Add selectSingle

b04b88b

Clean up and add to test execution :/

b9fbd02

Import library's selectFirst rather than re-export it from `persist…

0d266aa

…ent`

ibarrae added 2 commits June 2, 2021 09:48

Add haddock and update test name

7d55bfb

Add missing comments for haddock :)

2780c86

parsonsmatt requested changes Jun 2, 2021

View reviewed changes

ibarrae added 3 commits June 2, 2021 11:29

Rename to selectOne and add @SInCE

9693f21

Bump version number

206fb82

Add missing refs for table function

b1c6fc0

ibarrae marked this pull request as ready for review June 2, 2021 16:34

ibarrae requested a review from parsonsmatt June 2, 2021 16:35

ibarrae changed the title ~~Add selectSingle~~ Add selectOne Jun 2, 2021

belevy reviewed Jun 15, 2021

View reviewed changes

Update to experimental syntax

7622e68

ibarrae requested a review from belevy June 16, 2021 20:45

parsonsmatt approved these changes Jun 17, 2021

View reviewed changes

belevy approved these changes Jun 17, 2021

View reviewed changes

parsonsmatt merged commit 3312804 into bitemyapp:master Jun 17, 2021

ibarrae deleted the add-select-first branch July 12, 2021 19:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `selectOne` #265

Add `selectOne` #265

ibarrae commented Jun 2, 2021 •

edited

Loading

belevy commented Jun 2, 2021

ibarrae commented Jun 2, 2021 •

edited

Loading

belevy commented Jun 2, 2021

parsonsmatt commented Jun 2, 2021

parsonsmatt left a comment

parsonsmatt Jun 2, 2021

parsonsmatt Jun 2, 2021

parsonsmatt Jun 2, 2021

ibarrae Jun 2, 2021

parsonsmatt Jun 2, 2021

parsonsmatt Jun 2, 2021

ibarrae commented Jun 2, 2021

belevy Jun 15, 2021

ibarrae Jun 16, 2021

parsonsmatt left a comment

parsonsmatt Jun 17, 2021

parsonsmatt Jun 17, 2021

parsonsmatt commented Jun 17, 2021

	import Database.Esqueleto.Internal.PersistentImport hiding (selectFirst)
	import Database.Esqueleto.Internal.PersistentImport

Add selectOne #265

Add selectOne #265

Conversation

ibarrae commented Jun 2, 2021 • edited Loading

belevy commented Jun 2, 2021

ibarrae commented Jun 2, 2021 • edited Loading

belevy commented Jun 2, 2021

parsonsmatt commented Jun 2, 2021

parsonsmatt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ibarrae commented Jun 2, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

parsonsmatt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

parsonsmatt commented Jun 17, 2021

Add `selectOne` #265

Add `selectOne` #265

ibarrae commented Jun 2, 2021 •

edited

Loading

ibarrae commented Jun 2, 2021 •

edited

Loading