ClickHouse / ClickHouse

ClickHouse® is a real-time analytics DBMS
https://clickhouse.com
Apache License 2.0
37.54k stars 6.9k forks source link

[Collection] Quirky clickhouse-format behaviours #48140

Open murfel opened 1 year ago

murfel commented 1 year ago

Creating an umbrella ticket to collect all correct but somewhat inconsistent or inconvenient behaviours of clickhouse-format, whether it's hiliting, line breaks, or excessive parenthesis.

Issues from here could be used as a good first issue or intern tasks (linking in #42194) for those with a passion for UX and AST visitors.

murfel commented 1 year ago

Funny parentheses hiliting

Analyze in which cases parentheses are hilited, find a semantical meaning behind the hilite (there could be none, or it could be inconsistent), see if the user understands that semantic and if it does help to understand the query.

Example 1
clickhouse format --hilite --query "CREATE DICTIONARY somename (Name ClickHouseDataType DEFAULT '' EXPRESSION rand64()) SOURCE(HTTP(url 'http://[::1]/os.tsv' format 'TabSeparated' credentials(user 'user' password 'password') headers(header(name 'API-KEY' value 'key'))))"

The parentheses of CREATE DICTIONARY, SOURCE, and HTTP are not hilited. However, the parentheses within HTTP being FunctionWithKeyValueArguments consisting of ASTPairs are hilited.

https://github.com/ClickHouse/ClickHouse/blob/8d72f75556bbbb7dbe5675324bb3e00634480492/src/Parsers/ASTFunctionWithKeyValueArguments.cpp#L30

image

Example 2

The parentheses are hilited, though I'm not sure what's the semantical meaning behind this. CREATE TABLE t AS (SELECT 1) COMMENT 'hello'

https://github.com/ClickHouse/ClickHouse/blob/f36ad317516fc50676be994d7bbd28bd32f22422/src/Parsers/ASTCreateQuery.cpp#L440-L447

murfel commented 1 year ago

Excessive parentheses? Funny parentheses hiliting?

CREATE INDEX idx_table_name ON table_name((column1, column2)) TYPE Int8 GRANULARITY 1

image

UnamedRus commented 1 year ago

Incorrect query

clickhouse-format --query "SELECT 1 SETTINGS additional_table_filters={'table': '1'}";
SELECT 1
SETTINGS additional_table_filters = [('table', '1')]
❯ clickhouse-local -mn
ClickHouse local version 23.3.1.1881.

LAPTOP-. :) SELECT 1
                    SETTINGS additional_table_filters = [('table', '1')];

Syntax error: failed at position 46 ('[') (line 2, col 37):

SELECT 1
SETTINGS additional_table_filters = [('table', '1')];

Expected one of: DEFAULT, TRUE, FALSE, function, compound identifier, list of elements, identifier, literal or map, literal, NULL, number, Bool, true, false, string literal, token, OpeningCurlyBrace
murfel commented 1 year ago

Inconsistent hiliting for database/table

It seems a bit strange that the database and table are hilited as keywords, instead of identifiers:

https://github.com/ClickHouse/ClickHouse/blob/611f014082c82482d31b61376b8278db3ec56f21/src/Parsers/ASTCheckQuery.h#L39

And here not hilited at all:

https://github.com/ClickHouse/ClickHouse/blob/644d83653d4a1b520b6e43bf504c47904cf89766/src/Parsers/ASTAlterQuery.cpp#L607