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

Change FROM clause formatting (make it more human friendly) #24151

Merged
merged 2 commits into from
May 21, 2021

Conversation

azat
Copy link
Collaborator

@azat azat commented May 15, 2021

Changelog category (leave one):

  • Improvement

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Fix trailing whitespaces in FROM clause with subqueries in multiline mode, and also changes the output of the queries slightly in a more human friendly way.

Detailed description / Documentation draft:
Before:

$ clickhouse-format <<<'select * from system.one, (select * from system.one)'
SELECT *
FROM system.one
,
(
    SELECT *
    FROM system.one
)

After:

$ clickhouse-format <<<'select * from system.one, (select * from system.one)'
SELECT *
FROM system.one,
(
    SELECT *
    FROM system.one
)

NOTE: should it be marked with Backward Incompatible Change? (since it changes the output that someone may relay on)

@robot-clickhouse robot-clickhouse added the pr-improvement Pull request with some product improvements label May 15, 2021
@alexey-milovidov
Copy link
Member

should it be marked with Backward Incompatible Change?

It's alright, we shouldn't guarantee how queries will be formatted.

@alexey-milovidov alexey-milovidov self-assigned this May 15, 2021
@azat azat force-pushed the format-FROM-clause branch 5 times, most recently from f2102a6 to 5642524 Compare May 16, 2021 05:39
@azat azat marked this pull request as draft May 16, 2021 10:19
@azat azat marked this pull request as ready for review May 16, 2021 11:44
@kitaisreal
Copy link
Collaborator

@azat there are some issues with functional statelss tests. Are they related to changes ?

@azat
Copy link
Collaborator Author

azat commented May 18, 2021

there are some issues with functional statelss tests. Are they related to changes ?

AFAICS - no

  • there are few failures due to the test takes too long (marked them as long)
  • there is one strange failure - 01414_push_predicate_when_contains_with_clause (will take a look)
  • and unrelated 00953_zookeeper_suetin_deduplication_bug

@azat
Copy link
Collaborator Author

azat commented May 18, 2021

Functional stateless tests flaky check (address) — Timeout, fail: 67, passed: 2682

01414_push_predicate_when_contains_with_clause - #24259

@azat azat force-pushed the format-FROM-clause branch 2 times, most recently from aa1bce3 to 44c9f08 Compare May 19, 2021 04:19
azat added 2 commits May 20, 2021 21:04
Fix trailing whitespaces in FROM/IN clause with subqueries in multiline
mode, and also changes the output of the queries slightly in a more
human friendly way.

Before:

    $ clickhouse-format <<<'select * from system.one, (select * from system.one)'
    SELECT *
    FROM system.one
    ,
    (
        SELECT *
        FROM system.one
    )

After:

    $ clickhouse-format <<<'select * from system.one, (select * from system.one)'
    SELECT *
    FROM system.one,
    (
        SELECT *
        FROM system.one
    )

v2: Fix subqueries formatting in a different way
v3: Adjust *.reference in tests
v4: Fix modernize-loop-convert in ASTTablesInSelectQuery
@alexey-milovidov alexey-milovidov merged commit 04e3dde into ClickHouse:master May 21, 2021
@azat azat deleted the format-FROM-clause branch May 21, 2021 03:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-improvement Pull request with some product improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants