Skip to content

Remove trailing whitespaces from formatted queries in some cases#11325

Merged
alexey-milovidov merged 16 commits intomasterfrom
avoid-trailing-whitespaces-in-some-cases
Jun 15, 2020
Merged

Remove trailing whitespaces from formatted queries in some cases#11325
alexey-milovidov merged 16 commits intomasterfrom
avoid-trailing-whitespaces-in-some-cases

Conversation

@alexey-milovidov
Copy link
Copy Markdown
Member

Changelog category (leave one):

  • Improvement

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Remove trailing whitespaces from formatted queries in clickhouse-client or clickhouse-format in some cases.

Detailed description / Documentation draft:
Trailing whitespaces in queries copy-pasted from terminal look ugly.

@blinkov blinkov added the pr-improvement Pull request with some product improvements label May 31, 2020
{
if (separator)
settings.ostr << separator;
settings.ostr << ' ';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

FWIW TSVRaw uses formatImpl and it will still has trailing whitespaces

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ok, I will fix it also.

@KochetovNicolai KochetovNicolai self-assigned this Jun 4, 2020
CREATE VIEW test_01083.view\n(\n `n` Int64\n) AS\nSELECT toInt64(n) AS n\nFROM \n(\n SELECT toString(n) AS n\n FROM test_01083.merge\n WHERE _table != \'qwerty\'\n ORDER BY _table ASC\n)\nUNION ALL\nSELECT *\nFROM test_01083.file
CREATE DICTIONARY test_01083.dict\n(\n `n` UInt64, \n `col` String DEFAULT \'42\'\n)\nPRIMARY KEY n\nSOURCE(CLICKHOUSE(HOST \'localhost\' PORT 9440 SECURE 1 USER \'default\' TABLE \'url\' DB \'test_01083\'))\nLIFETIME(MIN 0 MAX 1)\nLAYOUT(CACHE(SIZE_IN_CELLS 1))
CREATE VIEW test_01083.view\n(`n` Int64\n) AS\nSELECT toInt64(n) AS n\nFROM \n(\n SELECT toString(n) AS n\n FROM test_01083.merge\n WHERE _table != \'qwerty\'\n ORDER BY _table ASC\n)\nUNION ALL\nSELECT *\nFROM test_01083.file
CREATE DICTIONARY test_01083.dict\n(\n \n `n` UInt64,\n \n `col` String DEFAULT \'42\'\n)\nPRIMARY KEY n\nSOURCE(CLICKHOUSE(HOST \'localhost\' PORT 9440 SECURE 1 USER \'default\' TABLE \'url\' DB \'test_01083\'))\nLIFETIME(MIN 0 MAX 1)\nLAYOUT(CACHE(SIZE_IN_CELLS 1))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks suspicious.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed.

3
4
CREATE TABLE memory_01069.file\n(\n `n` UInt8\n)\nENGINE = File(\'CSV\')
CREATE TABLE memory_01069.file\n(`n` UInt8\n)\nENGINE = File(\'CSV\')
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I suppose we need to remove \n before ( and after ). Or restore previous behaviour.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Now you can see that the test results only differ in trailing whitespaces, as expected.

@alexey-milovidov alexey-milovidov merged commit 67b9247 into master Jun 15, 2020
@alexey-milovidov alexey-milovidov deleted the avoid-trailing-whitespaces-in-some-cases branch June 15, 2020 16:49
azat added a commit to azat/ClickHouse that referenced this pull request Jun 17, 2020
After ClickHouse#11325 trailing whitespaces has been removed for data skipping
indicies, and it may be different, if you have multiple skip indices,
and in this case new server will not load such tables, because metadata
will be different.

Fix this by re-parse metadata in zookeeper.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-docs-needed pr-improvement Pull request with some product improvements usability

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants