Test using the prettyprinter library for rendering SQL

[csasarak]

This is a test based on the discussion in #435. My benchmarks (on a relatively simple query) show it to be a bit faster than the old one. I want to try using the fuse function from prettyprinter to see if that can help since many of the printing functions add a few relatively small things to the append.

It's pretty rough, and if this is a direction we're interested in going I'll need some guidance on design.

[csasarak]

Actually, for fun I tried using this in my real application and it's a bit quicker. I will try to think of some better benchmarks than what I was testing with before.

[tomjaguarpaw]

Can you explicitly import <> from Data.Monoid so it works with old GHCs?

if this is a direction we're interested in going I'll need some guidance on design.

This definitely looks like the right approach. I'm happy to provide guidance. What would you like guidance on?

I would suggest a few type synonyms and helper functions so as to reduce diff churn, for example

• type Doc = Data.Text.Prettyprint.Doc SqlStatement
• empty = mempty
• text = fromString

Basically let's try to get the diff to be as small as possible.

[csasarak]

@tomjaguarpaw I meant exactly what you've done, thanks! I want to make sure this change doesn't break things so I tried to limit the text stuff to its own set of functions in the RunQuery module. Do you think that the types in Sql should have String changed to Text? That sounds like it could be a breaking change, so I didn't do it.

One question I have is why Opaleye.Internal.HaskellDB.Sql.Print and Opaleye.Internal.Print are separate? It looks like one is only for Selects and the other has Update/Delete/Insert. I ask because I'd like to make an explicit export list to help GHC inline a bit better, which might be improved by combining the two modules so that less needs to be exposed.

I will incorporate your suggestions, thank you!

[tomjaguarpaw]

The two modules are separate because the Internal.HaskellDB one was lifted directly from the haskelldb package and the Internal one was written fresh. I'm not against merging them if it improves performance.

[tomjaguarpaw]

You can change the types from String to Text if you like. I can't imagine that that would help performance much though, because they're very small.

One thing we need to consider is what is actually the correct type of thing to create in the first place. runQueryExplicit uses postgresql-simple's queryWith_. The latter uses Query which is just a newtyped ByteString. Ideally then we would be generating ByteStrings directly. Unfortunately it doesn't seem that prettyprinter supports that. We'd have to generate Text and then UTF8-encode the Text.

[csasarak]

I may give that a try next then.

I have reduced the diff size and pushed, though some things I kept changed into forms that match the way prettyprinter works a bit better.

[tomjaguarpaw]

It would still be good to see empty = mempty.

[tomjaguarpaw]

In fact if you can import Prelude hiding (++) and define (++) = (<>) then I think we're doing even better. Ideally we would have zero changes to the body of the module at all.

[csasarak]

You can change the types from String to Text if you like. I can't imagine that that would help performance much though, because they're very small.

You're right here, probably not much gain to be had.

One thing we need to consider is what is actually the correct type of thing to create in the first place. runQueryExplicit uses postgresql-simple's queryWith_. The latter uses Query which is just a newtyped ByteString. Ideally then we would be generating ByteStrings directly. Unfortunately it doesn't seem that prettyprinter supports that. We'd have to generate Text and then UTF8-encode the Text.

That is true, and its the approach I've taken now. The nice thing is that now that we have this code creating a prettyprinter Doc we can output it to other formats. A next step would actually be to add a ByteString encoder to the prettyprinter package IMO. If you look at the module hierarchy here: https://hackage.haskell.org/package/prettyprinter-1.3.0, there are output modules for several formats. If we had one for ByteString, we could avoid the generate-text-then-encodeUtf8 pattern.

[csasarak]

I've done my best to reduce the number of changes made to the body of those modules. I have also opened an issue with prettyprinter to gauge their interest in merging a bytestring renderer, unless you think this is the wrong approach.

[tomjaguarpaw]

I pushed https://github.com/tomjaguarpaw/haskell-opaleye/tree/prettyprinter-test. It makes zero changes in the body of the printing modules. Now that we've got to that point feel free to build on top and go crazy optimising as much as you can!

[csasarak]

Thank you! I will see what I can do.