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

Add FORMAT statement #6725

Closed
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
3 participants

@kokosing kokosing changed the title from Leverage indentation in ExpressionFormatter to Add FORMAT statement Nov 30, 2016

@electrum

This comment has been minimized.

Show comment
Hide comment
@electrum

electrum Nov 30, 2016

Member

I need to think about this more, but my initial thought is that this shouldn't be a SQL language feature. There are lots of tools that can format SQL queries. Ours isn't particularly good, nor is it configurable. Once you go down this route, people will continually ask for customization. Also, the output format isn't specified and can change from release to release.

I'd rather have this be a separate command line tool, or added to the CLI (either as a command line option or a special command like "help" or "exit").

For example on possible customizations, see what IntelliJ supports:

screen shot 2016-11-30 at 10 45 59 am

Member

electrum commented Nov 30, 2016

I need to think about this more, but my initial thought is that this shouldn't be a SQL language feature. There are lots of tools that can format SQL queries. Ours isn't particularly good, nor is it configurable. Once you go down this route, people will continually ask for customization. Also, the output format isn't specified and can change from release to release.

I'd rather have this be a separate command line tool, or added to the CLI (either as a command line option or a special command like "help" or "exit").

For example on possible customizations, see what IntelliJ supports:

screen shot 2016-11-30 at 10 45 59 am

@kokosing

This comment has been minimized.

Show comment
Hide comment
@kokosing

kokosing Dec 1, 2016

Member

@electrum Before doing thisI used some web formatters and they were poor. I remember some changed the semantic of couple TPC-DS queries. The only advantage I see of having this tool integrated with presto is that it completely understands presto SQL syntax which may not be true for externals formatter. Note lambda extension for example I have no idea how such external formatter can behave.

I can re-implement this as special CLI command or another CLI tool. I can even make it a 3rd party command line application. Your call.

Member

kokosing commented Dec 1, 2016

@electrum Before doing thisI used some web formatters and they were poor. I remember some changed the semantic of couple TPC-DS queries. The only advantage I see of having this tool integrated with presto is that it completely understands presto SQL syntax which may not be true for externals formatter. Note lambda extension for example I have no idea how such external formatter can behave.

I can re-implement this as special CLI command or another CLI tool. I can even make it a 3rd party command line application. Your call.

@kokosing

This comment has been minimized.

Show comment
Hide comment
@kokosing

kokosing Jan 30, 2017

Member

@electrum Recently there were EXPLAIN (TYPE VALIDATE merged, see #7020. What would you say about EXPLAIN (TYPE FORMAT)?

Member

kokosing commented Jan 30, 2017

@electrum Recently there were EXPLAIN (TYPE VALIDATE merged, see #7020. What would you say about EXPLAIN (TYPE FORMAT)?

@kokosing

This comment has been minimized.

Show comment
Hide comment
@kokosing

kokosing Feb 8, 2017

Member

Ok, I extracted #7329 from this PR. Also I created an external tool which I use from time to time here - https://github.com/prestodb-rocks/presto-query-formatter

Said above. I am closing this pull request.

Member

kokosing commented Feb 8, 2017

Ok, I extracted #7329 from this PR. Also I created an external tool which I use from time to time here - https://github.com/prestodb-rocks/presto-query-formatter

Said above. I am closing this pull request.

@kokosing kokosing closed this Feb 8, 2017

@kokosing kokosing deleted the Teradata:feature_format branch Mar 29, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment