QuasiQuoter Improvements #1256

parsonsmatt · 2021-04-27T23:53:08Z

This PR improves the QuasiQuoter. It no longer returns an EntityDef, instead it returns UnboundEntityDef. All of the compile-time type splicing responsibilities have been shifted over to mkPersist.

You can now refer to foreign keys defined outside of the module.

You can refer to entity IDs in an Id column (eg Id UserId now works).

Still need to fix the setting of foreign key references.

This PR also disables the MongoDB build. The reason is that breaking the cycle in the EmbedEntityDef is necessary, and is also quite difficult! It'd be nice to support MongoDB, but I'd rather just pause support for Mongo for now. Given that Persistent gets 4470 downlaods in the last 30 days and Mongo only has 115, I'm OK with temporarily dropping support.

I've added upper bounds constraints so folks won't be able to upgrade when this is released.

Consider this a call to action for additional maintainers for persistent-mongoDB!

Before submitting your PR, check that you've:

Documented new APIs with Haddock markup
Added @since declarations to the Haddock
Ran stylish-haskell on any changed files.
Adhered to the code style (see the .editorconfig file for details)

After submitting your PR:

Update the Changelog.md file with a link to your PR
Bumped the version number if there isn't an (unreleased) on the Changelog
Check that CI passes (or if it fails, for reasons unrelated to your change, like CI timeouts)

Fixes #1241

Fixes #1073

Fixes #994

Fixes #1042

FIxes #786

Fixes #1149

parsonsmatt · 2021-04-28T16:43:01Z

Currently we're getting a test failure where all foo SomeTableId columns are determined to be SqlInt64 by the QuasiQuoter, and this isn't getting fixed in post.

I think a fix is to have an UnboundFieldDef on UnboundEntityDef, and these are spliced up nice and right in post. That should make the threading behavior more clear.

parsonsmatt

Massive change. Wish this were a smaller PR but I just kept smashing into problems and fixing things.

Very excited to get this in. Will make it a lot easier to work on the QQ and add features now that we're being more direct about what we can provide.

I need to write a blog post on this whole technique. It's pretty amazing. A two phase Template Haskell splicing setup is really quite brilliant. But I think it may be possible to get it to a single phase...

parsonsmatt · 2021-05-03T22:34:02Z

cabal.project

@@ -2,7 +2,7 @@ packages:
  persistent
  persistent-sqlite
  persistent-test
-  persistent-mongoDB
+  -- persistent-mongoDB


mongo DB is out of commission for now.

the embed entity def stuff is really hard to get right

i think it'll be easy to do when i've got, like, more intelligent multicolumn fields

but for right now i'm just disabling it

parsonsmatt · 2021-05-03T22:37:03Z

persistent-sqlite/test/Database/Persist/Sqlite/CompositeSpec.hs

+{-# LANGUAGE DerivingStrategies #-}
+{-# OPTIONS_GHC -fno-warn-unused-top-binds #-}
+
+module Database.Persist.Sqlite.CompositeSpec where


Trying to pull out some stuff and better organize the test suite

parsonsmatt · 2021-05-03T22:38:13Z

persistent-test/src/CompositeTest.hs

 import Init


 -- mpsGeneric = False is due to a bug or at least lack of a feature in mkKeyTypeDec TH.hs
-share [mkPersist persistSettings { mpsGeneric = False }, mkMigrate "compositeMigrate", mkDeleteCascade persistSettings { mpsGeneric = False }] [persistLowerCase|
+share [mkPersist persistSettings { mpsGeneric = False }, mkMigrate "compositeMigrate"] [persistLowerCase|


mkDeleteCascade is gone, replaced with the cascade on fields directly

parsonsmatt · 2021-05-03T22:40:13Z

persistent-test/src/PersistentTest.hs

@@ -640,72 +640,3 @@ specsWith runDb = describe "persistent" $ do
      fieldComments nameField
        `shouldBe`
          Just "Fields should be documentable.\n"
-
-  describe "JsonEncoding" $ do


moved into a specific test module closer to home

parsonsmatt · 2021-05-03T22:41:11Z

persistent/Database/Persist/Class/PersistEntity.hs

@@ -105,7 +115,7 @@ class ( PersistField (Key record), ToJSON (Key record), FromJSON (Key record)
    -- | A meta operation to retrieve all the 'Unique' keys.
    persistUniqueKeys :: record -> [Unique record]
    -- | A lower level operation.
-    persistUniqueToFieldNames :: Unique record -> [(FieldNameHS, FieldNameDB)]
+    persistUniqueToFieldNames :: Unique record -> NonEmpty (FieldNameHS, FieldNameDB)


this is a great big change and it is so good

parsonsmatt · 2021-05-03T22:59:12Z

persistent-test/src/TreeTest.hs

@@ -60,7 +58,7 @@ specsWith runDb = describe "tree" $ do
                ConstraintNameHS "fkparent"
        it "has the right DB constraint name" $ do
            foreignConstraintNameDBName `shouldBe`
-                ConstraintNameDB "treesfkparent"
+                ConstraintNameDB "treefkparent"


i should figure out how to unbork that :\

parsonsmatt · 2021-05-03T23:01:01Z

persistent/Database/Persist/Sql/Util.hs

-entityColumnNames ent conn =
-     (if hasNaturalKey ent
-      then [] else [connEscapeFieldName conn . fieldDB $ getEntityId ent])
-  <> map (connEscapeFieldName conn . fieldDB) (getEntityFields ent)


this was entirely equivalent? lol

parsonsmatt · 2021-05-03T23:02:04Z

persistent/Database/Persist/TH.hs

+    --
+    -- @since 2.13.0.0
+    -> [UnboundEntityDef]
+    -> [UnboundEntityDef]


fortunately a lot of the new lines are, like, comments and indentation and spacing t make it easier to read

parsonsmatt · 2021-05-03T23:02:58Z

persistent/Database/Persist/TH.hs

-- look at every emFieldEmbed to see if it refers to an already seen EntityNameHS
-- so start with entityHaskell ent and accumulate embeddedHaskell em
-breakEntDefCycle :: EntityDef -> EntityDef
-breakEntDefCycle entDef =


EmbedEntityDef no longer has a cycle

need to destroy embedding no-normal things

parsonsmatt · 2021-05-03T23:04:03Z

persistent/Database/Persist/TH.hs

 -- @since 2.5.3
 parseReferences :: PersistSettings -> Text -> Q Exp
-parseReferences ps s = lift $
-    map (mkEntityDefSqlTypeExp embedEntityMap entityMap) noCycleEnts
+parseReferences ps s = lift $ parse ps s


now, honestly,

it would be kinda cool to have this do something a little smarter than merely lift :: [UnboundEntityDef] -> Q Exp

would love to see QQuasi in something like ParsecT _ Q

pbrisbin

I wish I had the domain knowledge to add a meaningful review here. The idea sounds great, and thanks for poking me for review. Maybe if you keep doing that, I will be able to add something meaningful some day :)

On the not meaningful side, I did just notice "Ran stylish-haskell on any changed files." in the checklist. I have a side-project that actually automates that, if you're interested: https://restyled.io. If you want any more details or help setting it up, let me know. It's all open source and in Haskell too.

parsonsmatt · 2021-05-04T17:07:34Z

@pbrisbin If you're still at Freckle, it'd be nice to know how these changes might affect y'all's codebase. I'll make an issue to get restyled working, that'd be great 😄

pbrisbin · 2021-05-04T17:24:13Z

If you're still at Freckle, it'd be nice to know how these changes might affect y'all's codebase

Oh yeah, totally appreciate that. Can you maybe link me to a specific part that shows the "user"-facing changes? Will we have to modify our existing entity definitions? I got the sense this would open up some new things, but not require any actual migration to them yet, but that could be wrong.

parsonsmatt · 2021-05-04T17:28:05Z

Yeah, that should be the case. The biggest differences are:

If you're writing your own thing that uses the QuasiQuoter, then the changes to that will be breaking. eg [peristLowerCase|...|] no longer spliced in a [EntityDef], it splices in a [UnboundEntityDef].
If you're using embedded entities extensively, or in ways that aren't covered by the test suite, then this may break your code. I'm not terribly confident that the tests there cover all possible uses.

pbrisbin · 2021-05-04T17:31:19Z

Cool, (2) won't impact us. For (1), all our definitions are of the form:

mkPersist sqlSettings [persistLowerCase|
Teacher sql=teachers
  -- ...
|]

So as long as mkPersist accepts whatever persistLowerCase will now splice in, we are unaffected.

* Internalize SqlBackend (#1225) * abstractification * abstractification * Internalize SqlBackend * suppor tother sql dbs * bounds * asdf * import data.monoid * limitoffset and backendspecificoverrides * use setter * getConnUpsertSql * formatting * sigh * clean warns * cabal formatting * merge resolve * lol * ok * update changelogs * one more [ci skip] * Implicit ID Column Configuration (#1234) * Implicit ID Column COnfiguration * PersistSettings is internal * start teasing out the module structure * move around, factor out the Names module * start enumerating types, make entitydef abstract * entity def abstraction * teasing out the EntityDef stuff * builds * testinggg * it works * it works * tidy up * sigh * i hate you * tidy * wrote test for mysql, need to set maxlen sigh * mysql test, need to be able to set maxlen * support mysql lmfao * whyyy * Deprecate mpsGeneric (#1250) * discoverEntities (#1253) * discover entities * remove fdescribe * changelog * yupo * remove error * Fix migrations (#1252) * Better migrations * why is the test failing * Columns are present in entityFields now, but the generated code is broken. * th specs work * fixed mkColumns * changelog entry * fix mongo * no idea why this is broken now * why on earth did this work * remove debug trace statements * typo * what no put that back in * Check for existence of entities before generating them (#1255) * wtf * hmmm * refactor and tidy * are foreign fields never right ?! * changelog * fix comments * dead code * Implement config for customising the FK name (#1244) * Implement config for customising the FK name * Update changelog * Tweak test description * Tweaks/better use of types * Review tweaks * Some initial post-review changes * Table name turned out to be EntityNameHS * Do the same thing but for the constraint * Expose more stuff * Some refactoring / cleanup * Fix changelog indentation * Tidy code layout * QuasiQuoter Improvements (#1256) * QQ now returns UnboundEntityDef * Relocate fixForeignKeysAll * deprecate some stuff, reorganize some code * ok, now we need to set sql types appropriately. * dodgy instances are banned * fuse away the EntityDefSqlTypeExp stuff * refactor to top level * fuse sqlTypeExp in there * fix Key vs Id stuff * still need to get the foreign key types right * hmmm * move to QuasiSpec * clean up tests * so close * ok but what if i don't fix foreign keys * wip * oh man please * getting closer... * make some tests * fix json and keyFromValueM * slightly more graceful handling * return dummy field for id, from persist values * got some tests passing * well sqlite works * pg tests running * what happened * hmm mongo is trashed maybe * bye mongo * ok for real bye mongo, for now at least * clean warns * asdf * drop GHC 8.2 support * sigh * lots of commments * Merge branch 'master' into persistent-2.13 (#1261) * Reexport PersistValue * fix json for MigrationOnly * Export onlyOneUniqueDef (fixes #1194) * use onlyOneUniqueDef in persistent-postgresql Co-authored-by: Dan Brooks <danbroooks@users.noreply.github.com>

parsonsmatt added 9 commits April 27, 2021 13:29

QQ now returns UnboundEntityDef

ef771dc

Relocate fixForeignKeysAll

f4d6670

deprecate some stuff, reorganize some code

f272767

ok, now we need to set sql types appropriately.

9e1d600

dodgy instances are banned

a954a73

fuse away the EntityDefSqlTypeExp stuff

f1549ad

refactor to top level

ab60f1a

fuse sqlTypeExp in there

ac2099e

fix Key vs Id stuff

54e2cb3

parsonsmatt added this to the 2.13 milestone Apr 27, 2021

parsonsmatt changed the base branch from master to persistent-2.13 April 27, 2021 23:53

still need to get the foreign key types right

b125d1f

parsonsmatt marked this pull request as draft April 28, 2021 00:07

hmmm

7be9529

parsonsmatt mentioned this pull request Apr 28, 2021

Implement config for customising the FK name #1244

Merged

7 tasks

parsonsmatt added 14 commits April 28, 2021 12:27

merge upstream

4d6774f

move to QuasiSpec

cc6a69a

clean up tests

0e036fc

so close

1cc7eda

ok but what if i don't fix foreign keys

dca2ee6

wip

a927740

oh man please

7d97535

getting closer...

1119a6c

make some tests

714c6e9

fix json and keyFromValueM

2e9a6b3

slightly more graceful handling

3580495

return dummy field for id, from persist values

85ee16a

got some tests passing

afa355b

well sqlite works

9ceb489

parsonsmatt marked this pull request as ready for review May 3, 2021 22:32

parsonsmatt added 2 commits May 3, 2021 16:56

clean warns

a0be82e

asdf

2084c71

parsonsmatt mentioned this pull request May 4, 2021

Foreign syntax doesn't recognize tables in other modules #1073

Closed

parsonsmatt added 3 commits May 4, 2021 07:01

drop GHC 8.2 support

f64e409

sigh

3531d82

lots of commments

e09f531

parsonsmatt commented May 4, 2021

View reviewed changes

parsonsmatt requested review from MaxGabriel, pbrisbin and snoyberg May 4, 2021 14:43

pbrisbin reviewed May 4, 2021

View reviewed changes

This was referenced May 4, 2021

Cannot create complex circular relationships between tables and datatypes #147

Closed

Migration with foreign key failing with two separate calls to runMigration #152

Closed

parsonsmatt merged commit bac761a into persistent-2.13 May 4, 2021

parsonsmatt deleted the matt/forgiving-quasi-quoter branch May 4, 2021 22:11

This was referenced May 4, 2021

persistent-template: Phantom self-reference is an error #729

Closed

Persistent TH generates splice so large that GHCI runs out labels #1262

Open

aschmois mentioned this pull request Jun 16, 2021

2.13 support for mongodb #1284

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

QuasiQuoter Improvements #1256

QuasiQuoter Improvements #1256

parsonsmatt commented Apr 27, 2021 •

edited

Loading

parsonsmatt commented Apr 28, 2021

parsonsmatt left a comment

parsonsmatt May 3, 2021

parsonsmatt May 3, 2021

parsonsmatt May 3, 2021

parsonsmatt May 3, 2021

parsonsmatt May 3, 2021

parsonsmatt May 3, 2021

parsonsmatt May 3, 2021

parsonsmatt May 3, 2021

parsonsmatt May 3, 2021

parsonsmatt May 3, 2021

pbrisbin left a comment

parsonsmatt commented May 4, 2021

pbrisbin commented May 4, 2021

parsonsmatt commented May 4, 2021

pbrisbin commented May 4, 2021

QuasiQuoter Improvements #1256

QuasiQuoter Improvements #1256

Conversation

parsonsmatt commented Apr 27, 2021 • edited Loading

parsonsmatt commented Apr 28, 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

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

pbrisbin left a comment

Choose a reason for hiding this comment

parsonsmatt commented May 4, 2021

pbrisbin commented May 4, 2021

parsonsmatt commented May 4, 2021

pbrisbin commented May 4, 2021

parsonsmatt commented Apr 27, 2021 •

edited

Loading