Kusto phase1

[kashwy]

Changelog category (leave one):

• New Feature

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

This is the initial implement of Kusto Query Language. (MVP)

Add Support to Kusto Query Language (KQL) as mentioned in this issue:
#36806

In this PR, we support the following features .

• Tabular expression statements
• Limit returned results
• Select Column (basic project)
• sort, order
• Perform string equality operations
• Filter using a list of elements
• Filter using common string operations
• Aggregate by columns
• Basic aggregate functions

We parse KQL queries to ClickHouse ASTs, use a setting called sql_dialect to switch between ClickHouse and KQL:
By default sql_dialect = 'clickhouse'
set sql_dialect = 'kusto'
Switch to parse KQL
set sql_dialect = 'clickhouse'
switch back to parse ClickHouse SQL.

Information about CI checks: https://clickhouse.com/docs/en/development/continuous-integration/

[yakov-olkhovskiy]

Overall approach with translation into SQL seems fine, though maybe not very efficient in the future for some KQL specific features.
Also I see some issues with expressions sequencing - for example expression t1 | take 3 | order by id will be evaluated to the same SQL as t1 | order by id | take 3 - apparently you are trying to combine everything into one SELECT which is not always correct...

[yakov-olkhovskiy]

let's make it just "dialect"

[tavplubix]

This feature looks quite marginal, so I would prefer to use longer setting name. We could use just dialect setting of type Enum (not String), but we don't have other dialects for now (and I'm not sure we will have more dialects in the near future). What do you think about something like use_kusto_query_language_dialect of type Bool?

[yakov-olkhovskiy]

@tavplubix Bool will exclude any other dialects in the future if we decide to add any - it should be mutually exclusive...

[kashwy]

there is another PR use the dialect.:
https://github.com/ClickHouse/ClickHouse/pull/36308/files
so there will be more dialects

[tavplubix]

Ok, so it makes sense to name it just dialect, but it must be Enum, not String

[kashwy]

Ok, so it makes sense to name it just dialect, but it must be Enum, not String
sure

[kashwy]

we changed the dialect to Enum.

[kashwy]

Overall approach with translation into SQL seems fine, though maybe not very efficient in the future for some KQL specific features. Also I see some issues with expressions sequencing - for example expression t1 | take 3 | order by id will be evaluated to the same SQL as t1 | order by id | take 3 - apparently you are trying to combine everything into one SELECT which is not always correct...

yes it's not efficient for some KQL specific features, according the concept of KQL, output of a pipeline is the input of next pipeline, so each pipeline will generate a query, I am not sure if this is efficient in ClickHouse when KQL has a lot of pipe

[yakov-olkhovskiy]

yes it's not efficient for some KQL specific features, according the concept of KQL, output of a pipeline is the input of next pipeline, so each pipeline will generate a query, I am not sure if this is efficient in ClickHouse when KQL has a lot of pipe

ClickHouse will try to optimize of course, but it's not always possible I guess. But for simple cases like this it can be done in KQL parser, so t1 | take 3 | order by id suppose to translate into select with sub-select

[kashwy]

yes it's not efficient for some KQL specific features, according the concept of KQL, output of a pipeline is the input of next pipeline, so each pipeline will generate a query, I am not sure if this is efficient in ClickHouse when KQL has a lot of pipe

ClickHouse will try to optimize of course, but it's not always possible I guess. But for simple cases like this it can be done in KQL parser, so t1 | take 3 | order by id suppose to translate into select with sub-select

yes, we can do for this case

[kashwy]

@kashwy will review and test it soon

thanks

[yakov-olkhovskiy]

I don't think it's correct:

ip-172-31-42-195.us-east-2.compute.internal :) tbl

SELECT *
FROM tbl

┌─a─┬─b─┐
│ 1 │ 1 │
│ 1 │ 2 │
│ 2 │ 1 │
│ 2 │ 2 │
└───┴───┘
4 rows in set. Elapsed: 0.009 sec.

ip-172-31-42-195.us-east-2.compute.internal :) tbl | order by b | limit 2

SELECT *
FROM
(
    SELECT *
    FROM tbl
    LIMIT 2
)
ORDER BY b DESC

┌─a─┬─b─┐
│ 1 │ 2 │
│ 1 │ 1 │
└───┴───┘
2 rows in set. Elapsed: 0.018 sec.

ip-172-31-42-195.us-east-2.compute.internal :) tbl | limit 2 | order by b

SELECT *
FROM
(
    SELECT *
    FROM tbl
    LIMIT 2
)
ORDER BY b DESC

┌─a─┬─b─┐
│ 1 │ 2 │
│ 1 │ 1 │
└───┴───┘
2 rows in set. Elapsed: 0.018 sec.

I see at least two discrepancies here - default order should be ASC I think, and both KQL expressions are evaluated to the same SQL, which is not correct - the first one should be SELECT * FROM tbl ORDER BY b ASC LIMIT 2

[kashwy]

@yakov-olkhovskiy thanks for the test, default for order in KQL is desc. I will fix the subquery for the limit.

[kashwy]

Hi, @yakov-olkhovskiy I have updated the PR, can you do a test and review again ?

Thanks

[kashwy]

Hi @yakov-olkhovskiy , did you get chance to test and review ?

Thanks.

[yakov-olkhovskiy]

I will check it soon

[kashwy]

@yakov-olkhovskiy I updated the PR according to your suggestions.

Thanks

[yakov-olkhovskiy]

@kashwy will review it soon

[yakov-olkhovskiy]

looks good to me

[kashwy]

looks good to me

@yakov-olkhovskiy thank you very much.

is it able to merge ?

[yakov-olkhovskiy]

@kashwy merged
for future - these tests don't look very reliable - they will fail if anything will be changed in the query printout - maybe try to find a better way to test

[kashwy]

thanks, we have added functional test in our phase2