Expose Structured Mock Migrations by mercury-kuba · Pull Request #1600 · yesodweb/persistent

mercury-kuba · 2025-07-01T21:12:51Z

Context

We'd like to be able to generate a structured representation of the proposed Postgres migrations when mocking migrations, instead of only having access to the raw text of statements.

This PR adds migrateStructured to Database.Persist.Postgresql.Internal to generate AlterDB data, which the existing implementation then converts to text as before.

It also modifies the structure of AlterDB(AddTable) to store structured information about the table that we're generating, instead of the raw text of the DB statements.

This PR does not change any semantic behavior of the existing system, and only moves code around into Internal.

I've run the Postgres test suite locally and it fully passed.

Checklist

Before submitting your PR, check that you've:

Documented new APIs with Haddock markup
Added @since declarations to the Haddock
Ran fourmolu on any changed files (restyled will do this for you, so
accept the suggested changes if it makes them)
Adhered to the code style (see the .editorconfig and fourmolu.yaml files 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)

mercury-kuba · 2025-07-01T21:13:17Z

persistent-postgresql/Database/Persist/Postgresql.hs

@@ -28,13 +28,10 @@ module Database.Persist.Postgresql
    ( withPostgresqlPool
    , withPostgresqlPoolWithVersion
    , withPostgresqlPoolWithConf
-


A lot of these are due to Fourmolu auto-formatting.

mercury-kuba · 2025-07-01T21:18:31Z

persistent-postgresql/Database/Persist/Postgresql/Internal.hs

+--
+-- @since 2.17.0.0
+data AlterDB
+    = AddTable EntityNameDB EntityIdDef [Column]


This constructor has changed to store table information instead of the raw text.

mercury-kuba · 2025-07-01T21:19:27Z

persistent-postgresql/Database/Persist/Postgresql/Internal.hs

+    idtxt =
+        case entityId of
+            EntityIdNaturalKey pdef ->
+                T.concat
+                    [ " PRIMARY KEY ("
+                    , T.intercalate "," $ map (escapeF . fieldDB) $ NEL.toList $ compositeFields pdef
+                    , ")"
+                    ]
+            EntityIdField field ->
+                let
+                    defText = defaultAttribute $ fieldAttrs field
+                    sType = fieldSqlType field
+                 in
+                    T.concat
+                        [ escapeF $ fieldDB field
+                        , maySerial sType defText
+                        , " PRIMARY KEY UNIQUE"
+                        , mayDefault defText
+                        ]
+    rawText =
+        T.concat
+            -- Lower case e: see Database.Persist.Sql.Migration
+            [ "CREATe TABLE " -- DO NOT FIX THE CAPITALIZATION!
+            , escapeE name
+            , "("
+            , idtxt
+            , if null nonIdCols then "" else ","
+            , T.intercalate "," $ map showColumn nonIdCols
+            , ")"
+            ]


We move the code that generates the AddTable migration text down here, but it's the same logic.

parsonsmatt · 2025-07-01T21:52:23Z

oof i should have just mass formatted everything in one commit instead of doing it incrementally. apologies

parsonsmatt

Versions should be 2.17.1.0.

I have a few questions/refactors- if we're exposing things (even internally) it may be nice to make these refactors ahead of time. Nothing blocking though.

The more structured representations are very nice!

parsonsmatt · 2025-07-01T21:57:04Z

persistent-postgresql/Database/Persist/Postgresql/Internal.hs


+-- | Indicates whether a Postgres Column is safe to drop.
+--
+-- @since 2.17.0.0


This is going to go out in at least 2.17.1.0

Also, if we're exposing it, let's newtype it, or make it a separate datatype.

Both are done.

parsonsmatt · 2025-07-01T22:00:46Z

persistent-postgresql/Database/Persist/Postgresql/Internal.hs

+    = ChangeType Column SqlType Text
+    | IsNull Column
+    | NotNull Column
+    | Add' Column


Is this AddColumn?

Updated to AddColumn

parsonsmatt · 2025-07-01T22:01:06Z

persistent-postgresql/Database/Persist/Postgresql/Internal.hs

+    | Drop Column SafeToRemove
+    | Default Column Text
+    | NoDefault Column
+    | Update' Column Text


Would also like to see a more descriptive name on here

Updated to UpdateNullToValue

parsonsmatt · 2025-07-01T22:01:47Z

persistent-postgresql/Database/Persist/Postgresql/Internal.hs

+--
+-- @since 2.17.0.0
+data AlterTable
+    = AddUniqueConstraint ConstraintNameDB [FieldNameDB]


Is it valid to have [] here? Or can we do NonEmpty?

Fixed for AddReference - it's a little more annoying for AddUniqueConstraint. But I don't think it's valid to have it empty here.

ok, we can just do followup issue then

parsonsmatt · 2025-07-01T22:02:33Z

persistent-postgresql/Database/Persist/Postgresql/Internal.hs

+    -> EntityDef
+    -> IO (Either [Text] [AlterDB])
+mockMigrateStructured allDefs entity = do
+    case partitionEithers [] of


I think you need to modify this :)

Not sure why that was there, but fixed: #1600 (comment)

mercury-kuba · 2025-07-01T22:24:12Z

persistent-postgresql/Database/Persist/Postgresql.hs

-mockMigrate allDefs _ entity = fmap (fmap $ map showAlterDb) $ do
-    case partitionEithers [] of
-        ([], old'') -> return $ Right $ migrationText False old''
-        (errs, _) -> return $ Left errs


This is where the partitionEithers is coming from - not sure why it was doing this, but I guess I can clean it up.

mercury-kuba · 2025-07-02T18:14:44Z

Ran the tests, here's the end of the output:

ERATED ALWAYS AS (5) STORED)
  should read a generated column [✔]
  should support adding or removing generation expressions from columns [ ]Migrating: CREATe TABLE "gen_migrate_test"("id" SERIAL8  PRIMARY KEY UNIQUE,"sickness" INT8 NOT NULL GENERATED ALWAYS AS (3) STORED,"cromulence" INT8 NOT NULL)
  should support adding or removing generation expressions from columns [✔]

Finished in 2.4905 seconds
283 examples, 0 failures

parsonsmatt · 2025-07-02T23:05:37Z

persistent-postgresql/Database/Persist/Postgresql/Internal.hs

+--
+-- @since 2.17.0.0
+data AlterTable
+    = AddUniqueConstraint ConstraintNameDB [FieldNameDB]


ok, we can just do followup issue then

parsonsmatt · 2025-07-02T23:07:30Z

persistent-postgresql/Database/Persist/Postgresql/Internal.hs

+    :: [EntityDef]
+    -> EntityDef
+    -> IO (Either [Text] [AlterDB])
+mockMigrateStructured allDefs entity = return $ Right migrationText


This feels odd to me - if we're always succeeding with Right, why not just do that? Same re IO. Is this suppose dto be doing more?

parsonsmatt · 2025-07-02T23:09:01Z

persistent-postgresql/Database/Persist/Postgresql/Internal.hs

+                "RESTRICT" ->
+                    Just Restrict
+                _ ->
+                    error $ "Unexpected value in parseCascade: " <> show txt


feels like this should throwError instead of error

parsonsmatt

Code all lgtm. Let's test in the work repo and ensure this does what we want before merging.

parsonsmatt · 2025-07-07T17:02:43Z

persistent/ChangeLog.md

+# 2.17.1.0
+
+* [#1600](https://github.com/yesodweb/persistent/pull/1600)
+    * Add `migrateStructured` to `Database.Persist.Postgresql.Internal`.
+      This allows you to access a structured representation of the proposed migrations
+      for use in your application.


Oh shoot. All the changes in this PR are actually in persistent-postgresql package. So this CHANGELOG entry needs to be relocated into persistent-postgresql/CHANGELOG.md and the relevant versioning followed.

Kuba Karpierz added 8 commits July 1, 2025 14:30

added mock structured

4625129

builds

206db47

re-export escape

7cfed0f

move show

8cf506a

started adding structured representation to AddTable

5a3c8af

make AddTable structured

df42045

remove unneeded lines

43afe36

ran fourmolu

c12707c

mercury-kuba commented Jul 1, 2025

View reviewed changes

added changelog

3f4594b

Kuba Karpierz added 2 commits July 1, 2025 17:56

Ran fourmolu again

f552151

manually add restyle

8692779

parsonsmatt reviewed Jul 1, 2025

View reviewed changes

Kuba Karpierz added 2 commits July 1, 2025 18:10

fix version @SInCE, add newtype to SafeToRemove

a117b2a

updated constructors to better names

d7170d4

mercury-kuba commented Jul 1, 2025

View reviewed changes

Kuba Karpierz added 9 commits July 1, 2025 18:27

remove unneeded partition

e982aa0

move to NEL for reference

a88fdfb

fourmolu

f0b001e

move structured migration to internal

49f1902

fourmolu

05d54d7

remove unused

7439e38

Removed redundant code from mock migration

77efee2

tests compile

f5b0912

Fourmolu

5a69456

mercury-kuba requested a review from parsonsmatt July 2, 2025 18:16

fixed typo

32a6d68

parsonsmatt reviewed Jul 2, 2025

View reviewed changes

Kuba Karpierz added 2 commits July 3, 2025 08:01

update type

85a22ae

fourmolu

9929a46

parsonsmatt approved these changes Jul 3, 2025

View reviewed changes

parsonsmatt reviewed Jul 7, 2025

View reviewed changes

Kuba Karpierz added 3 commits July 7, 2025 13:15

update changelog

abb291c

derived eq

1e62eb8

update cabal version

85702b4

parsonsmatt merged commit 5a9deb2 into yesodweb:master Jul 7, 2025
10 checks passed

Conversation

mercury-kuba commented Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Context

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

parsonsmatt commented Jul 1, 2025

Uh oh!

parsonsmatt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mercury-kuba commented Jul 2, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

parsonsmatt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mercury-kuba commented Jul 1, 2025 •

edited

Loading