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

feat: Support CREATE TABLE with ENGINE syntax for ClickHouse #78

Merged
merged 19 commits into from
Aug 11, 2023

Conversation

viladimiru
Copy link
Contributor

Closes #75

@viladimiru viladimiru self-assigned this Aug 10, 2023
@@ -52,7 +52,7 @@ TableDefinitionRightPart_EDIT
keywords.push({ value: 'LIKE', weight: 1 });
} else {
if (!$2) {
keywords.push({ value: 'PARTITIONED BY', weight: 12 });
keywords.push({ value: 'PARTITION BY', weight: 12 });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previously we had wrong suggestion value actually, because PARTITIONED BY token not exists and this suggestion break syntax

@@ -1298,6 +1298,48 @@ export const extendParser = function (parser) {
parser.yy.result.suggestTemplates = true;
}

parser.suggestEngines = function() {
Copy link
Contributor Author

@viladimiru viladimiru Aug 11, 2023

Choose a reason for hiding this comment

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

Actually we need to make revision of suggested engines, currently I included all engines which DataGrip supports, except PostgreSQL. Task #79

It's not easy actually, because some engine types haven's use cases and some other need to expand syntax

Copy link
Collaborator

@NikitaShkaruba NikitaShkaruba left a comment

Choose a reason for hiding this comment

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

I'm requesting changes because it's good to keep our public API consistent. Apart from that, amazing job!

@@ -136,3 +141,15 @@ export function parsePostgreSqlWithoutCursor(query: string): ParseResult {
// In order to truly complete a finished query, we need to add space to it like so "SELECT * FROM |".
return parsePostgreSql(query + ' ', '');
}


Copy link
Collaborator

Choose a reason for hiding this comment

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

We should remove this useless empty newline.

@@ -136,3 +141,15 @@ export function parsePostgreSqlWithoutCursor(query: string): ParseResult {
// In order to truly complete a finished query, we need to add space to it like so "SELECT * FROM |".
return parsePostgreSql(query + ' ', '');
}


export function parseClickhouse(queryBeforeCursor: string, queryAfterCursor: string): ParseResult {
Copy link
Collaborator

@NikitaShkaruba NikitaShkaruba Aug 11, 2023

Choose a reason for hiding this comment

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

In our websql we are writing databases in camel case ClickHouse. See the function above PostgreSql, not Postgresql. Let's make it consistent here also.

This should be fixed everywhere in this file.

export function parseClickhouseWithoutCursor(query: string): ParseResult {
// If our finished query is "SELECT * FROM|" and we try to parse it, parser thinks that we still haven't finished writing it and doesn't show some errors.
// In order to truly complete a finished query, we need to add space to it like so "SELECT * FROM |".
return parseClickhouse(query + ' ', '');
Copy link
Collaborator

@NikitaShkaruba NikitaShkaruba Aug 11, 2023

Choose a reason for hiding this comment

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

Let's move query + ' ' to a separate function. We're copypasting it and it's comment in 3 places

Copy link
Collaborator

@NikitaShkaruba NikitaShkaruba left a comment

Choose a reason for hiding this comment

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

Waow, let's merge it

@viladimiru viladimiru merged commit 759b642 into main Aug 11, 2023
3 checks passed
@viladimiru viladimiru deleted the 75-support-create-table-for-ch branch August 11, 2023 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support CREATE TABLE for ClickHouse
2 participants