Deparser: Add optional formatting/pretty printing

[lfittl]

TODO

• Review UNION handling (see complex_mattm.sql test case)
• Add additional test cases
• Review outputs for typical pg_dump
• Review comment handling (especially in regards to how the API is structured)
• Consider JOINs like other FROM elements (indent them like table names in FROM)
• Keep certain fields on one line (e.g. SELECT target list) up to a certain line length limit
• Introduce libpg_query convenience function for extracting comments from a source query (so we don't replicate this across the different bindings)
• Add option to emit commas at start of a line? (to help with reducing diffs in commits that change a command, like the INSERT at the end of Rails' db/structure.sql)
• Review TODO items mentioned in tests
• Make sure "make extract_source" pulls in the necessary functions (some were manually added to fix issues)
• Figure out how to pass comment emission state across statements (see comment from code review)
• Add code comment explaining parts/part groups/nesting levels (and specifically note how increase/decrease works with nesting levels and that they are supposed to be paired within a function)

Defer until later

• Improve handling of long lists generated by deparseExprList (e.g. see 21-in.psql and 03-many-columns.psql examples)
• Figure out a way to not nest the statement underneath EXPLAIN (e.g. see 12-explains.d/01-base.psql example)
• Improve handling of nested UNIONs and reduce unnecessary parenthesis (see 35-setops.psql example)
• Add option to emit long typenames (matching pg_dump)
• (Maybe not even needed) Option to emit pg_catalog schema qualification for built-in data types, functions? (is this important from a security perspective, or does the parser's special handling take precedence?)

[msakrejda]

I did not look at the code, but I reviewed the examples, and they all look pretty good. I agree with the TODOs, but I think OVER() is the only one I'd really miss.

[msakrejda]

Something like

SELECT a
FROM b
WHERE
    c IN (1, 2, 3)
    AND d IN (
        'something longer', 'something longer 2', 'something longer 3',
        'something longer 4', 'something longer 5', ''
    )

? Yeah, that might be nice.

[lfittl]

Yup. I will however defer this for now, it seems, based on doing some more investigation just now.

Figuring out how to make this work is non-trivial, and I'd like to get a first version of this code committed soon (and I don't think fixing this will break the API).

[msakrejda]

+1

[lfittl]

I may or may not address this before merging, but for now I've put this on the "could do in a follow-up" list.

[msakrejda]

+1

[lfittl]

FWIW, this is the same issue as the IN list case, so I'll fix those at the same time in a follow-up.

[msakrejda]

I still think 2 spaces would generally work better for this kind of formatting, but since it's configurable, it's not a big deal.

[lfittl]

I do find the 4 spaces helpful for tests since it makes the indentation levels clearly visible, but otherwise agree that in practical use 2 spaces is likely more convenient (and that's how we can use this in the app).

[lfittl]

I did not look at the code, but I reviewed the examples, and they all look pretty good. I agree with the TODOs, but I think OVER() is the only one I'd really miss.

Thank you for the review!

[msepga]

I'm curious about one condition (left a comment), but otherwise LGTM 👍