Skip to content

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.
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