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.

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 collation support #1492

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

RiugaBachi
Copy link

@RiugaBachi RiugaBachi commented May 11, 2023

This PR:

For context, these features were up-ported from a feature branch we have been using at Supercede. This branch was in turn branched off of #1488 , thus this PR will effectively subsume it since it hasn't been merged yet. Isaac (the PR author) has left the company since then, so I believe the original PR will remain stale.

Tests may be lacking; please do inform me if I overlooked anything test-wise!


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)

RiugaBachi and others added 15 commits May 11, 2023 02:18
Check for idempotency of collate= migrations
This lets the initial CREATE TABLE expression generate correctly with
the collate clause, eliding the need for an immediate automigration
to collate columns.
This new function allows for creating a `RawSqlite SqlBackend` manually
(without exposing the `RawSqlite` constructor) for code that wants to
open such a connection without having to opt-in to the resource
management of `withRawSqliteConnInfo` and co.

This is useful in my particular use case for creating a custom pool,
since I am not constrained by the `resource-pool` API re-exposed by
`persistent-sqlite`.
@RiugaBachi
Copy link
Author

RiugaBachi commented May 11, 2023

stylish-haskell errors out and truncates the file prematurely for me in Database.Persist.Postgresql because of lambda unicode characters in comments. Not sure how to work around this. There are open issues/PRs for this in stylish-haskell upstream.

I'm just going to mark this PR ready for review in any case; we can review formatting manually if there's no easy way to work around this issue.

@RiugaBachi RiugaBachi marked this pull request as ready for review May 11, 2023 17:15
@parsonsmatt parsonsmatt added this to the 2.15 milestone Sep 18, 2023
Copy link
Collaborator

@parsonsmatt parsonsmatt left a comment

Choose a reason for hiding this comment

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

Overall looks great!

This is unfortunately a breaking change and will need to wait for 2.15 to go out.

-- will associate with a particular field.
--
-- @since 2.15.0.0
newtype CollationName = CollationName { unCollatioName :: Text }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
newtype CollationName = CollationName { unCollatioName :: Text }
newtype CollationName = CollationName { unCollationName :: Text }

Comment on lines +14 to +18
* [#1488](https://github.com/yesodweb/persistent/pull/1488)
* Add `openRawSqliteConn` for creating `RawSqlite SqlBackend` connections
that aren't automatically cleaned-up.
* [#1459](https://github.com/yesodweb/persistent/pull/1459)
* Make use of `CautiousMigration` type alias for clarity.
Copy link
Collaborator

Choose a reason for hiding this comment

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

These changelog entries will need to be moved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants