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

Changes required for Opaleye 0.9.1.0 #165

Merged
merged 7 commits into from
Feb 14, 2022

Conversation

tomjaguarpaw
Copy link
Contributor

Opaleye 0.9.1.0 has fairly aggressive simplification of QueryArr internals. This PR is required for rel8 to build against it (0.9.1.0 is not internals-compatible with 0.9.0.0).

I took the opportunity to expose the rebind and distinct things you need in Opaleye's public API, so they won't break in future internal-only changes.

(I'm not completely certain that c47317e is correct. It doesn't seem to be tested. You can drop it if you find it's not correct. All the other commits are essential though.)

@ocharles
Copy link
Contributor

Thanks @tomjaguarpaw, I really appreciate your help when there are breaking changes (for us) in Opaleye! I've just bumped Haskell.nix so CI can run, and I've pinged @shane-circuithub to give me a hand reviewing.

@tomjaguarpaw
Copy link
Contributor Author

You're welcome. I tend to think that it's easier for me to make the fixes, since I know exactly the things that have changed in Opaleye.

@tomjaguarpaw
Copy link
Contributor Author

One thing to bear in mind when reviewing is that the new PrimQueryArr monoid composes in the opposite direction that the PrimQuery function builder used to compose, that is, the instance is

instance Semigroup PrimQueryArr where
  PrimQueryArr f1 <> PrimQueryArr f2 = PrimQueryArr (\lat -> f2 lat . f1 lat)

and what use to be written, for example, as

\lat -> Rebind True p lat . restrict e lat`

is now written in the "opposite" order

aRestrict e <> aRebind p

@ocharles
Copy link
Contributor

Good to know!

We've had a chat about this and the changes look good. We're going to deploy them at CircuitHub for the week just to double check, and if everything looks good I'll cut a new release on Friday. It's unfortunate I can't take enough confidence just from test results, but if anything comes up I can hopefully add tests in.

@shane-circuithub
Copy link
Contributor

This looks good to me, and I've tested the alignBy change and it seems to still work.

@shane-circuithub shane-circuithub merged commit ec80d9d into circuithub:master Feb 14, 2022
@tomjaguarpaw tomjaguarpaw deleted the opaleye-0.9.1.0 branch February 14, 2022 12:38
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