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

Render SQL statements in a single line. #103

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

k0001
Copy link
Contributor

@k0001 k0001 commented Aug 22, 2015

This change makes it easier to grep for queries in PostgreSQL's log, as well as copy-pasting said queries.

This change makes it easier to grep for queries in PostgreSQL's log, as
well as copy-pasting said queries.
@@ -200,7 +200,7 @@ table4columndata :: [(Column O.PGInt4, Column O.PGInt4)]
table4columndata = table4dataG

table6data :: [(String, String)]
table6data = [("xy", "a"), ("z", "a"), ("more text", "a")]
table6data = [("xy", "a"), ("z", "a"), ("more \n text", "a")]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Minor test to check whether newlines are preserved in strings. They are.

@tomjaguarpaw
Copy link
Owner

I'm not opposed to this but I'd prefer to hear from others what they think first. On the other hand I would happily merge a flag to choose SQL formatting style.

@k0001
Copy link
Contributor Author

k0001 commented Aug 22, 2015

Sure, let's discuss it. A couple of more comments/opinion about this:

  • This is how the SQL ends up currently looking like in the system log: https://gist.githubusercontent.com/k0001/df57c255c5ad85c3f839/raw/1efa89e65f3c6fcb46e09bed9bed242e90b731be/gistfile1.txt (SQL from the test suite). It's not very readable there.
  • Each line in the SQL is preceded with appropriate indentation whitespace, even if the log above doesn't show it. That whitespace can get large, and it needs to be sent across the network and afterwards parsed and discarded.
  • Adding a flag to choose the formatting means we probably need to add new variants of the runXxx functions as we don't have a practical way of providing a “global configuration” for these things (which is probably OK). I'd rather have this compact one-line format be the default when using runQuery and friends, but have the functions on Opaleye.Sql render the pretty-printed version useful for debugging.

@wraithm
Copy link
Contributor

wraithm commented Aug 24, 2015

It would be great if there were a flag of some sort, like Tom mentioned. Exposing a higher-order function might be a good way to implement this. I like the one-line format should be the default for postgresql consumption, like @k0001 said. I definitely use the pretty printing functions for debugging, and I really like having the indentation for those purposes.

@bergmark
Copy link
Contributor

I could see the ability to choose being useful for us. We'd just stick this in our Config so our library's interface wouldn't change regardless of opaleye's implementation.

@tomjaguarpaw
Copy link
Owner

I've been super busy, so got behind with PRs.

Renzo, ultimately I like your idea that what is sent over the wire should be on a single line. We should have two separate functions formatAndShowSQLNested and formatAndShowSQLSingleLine though, I think, so that people can choose between them.

@tomjaguarpaw
Copy link
Owner

Sorry this has been languishing. I've remained super busy.

I'm happy for the SQL that goes over the wire to be on a single line. I think we should preseve the existing behaviour of functions in Opaleye.Sql, but add formatAndShowSQLSingleLine and showSqlForPostgresSingleLineExplicit to be used by RunQuery.

Could you make those changes and rebase to HEAD? Then I'll be happy to merge.

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.

4 participants