Skip to content

Commit

Permalink
feat: move AggregateFunctionsSuggestion to a separate interface, refa…
Browse files Browse the repository at this point in the history
…ctor general parser create tests (#92)

* refactor: add misc fixes to generic/**/delete.test.ts

* refactor: migrate generic/jison/create test to typescript

* refactor: migrate generic/jison/create_database test to typescript

* refactor: migrate generic/jison/create_role test to typescript

* refactor: rename generic/jison/create_database, add todo

* refactor: migrate generic/jison/create_table test to typescript

* refactor: migrate generic/jison/create_view test to typescript

* refactor: add location tests to generic create tests

* docs: add CODE_CONVENTIONS.md

* refactor: make create tests more strict

* refactor: migrate alter tests to typescript

* feat: move TablesSuggestion to a separate variable, define DatabasesSuggestion, add todos

* refactor: add more todos to sql_main.jison

* refactor: fix readme

* fix: fix a test
  • Loading branch information
NikitaShkaruba authored Nov 30, 2023
1 parent 7402be4 commit 08e8c26
Show file tree
Hide file tree
Showing 25 changed files with 681 additions and 416 deletions.
14 changes: 14 additions & 0 deletions CODE_CONVENTIONS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# Code conventions

A set of rules all contributors should follow.
You can always link those rules in your PR without hesitation if you see that they are not followed.

## Testing

- Each test should only test a granular added piece of logic, and shouldn't test functionality of other's. E.g. if you are using OptionalIfNotExists, you shouldn't test if you get suggestions when writing '', 'IF ', 'IF NOT', just test if `IF NOT EXISTS` is suggesting, and it's enough.
- Each test file should contain at least:
- A test that checks a complete statement for errors
- A test that checks a complete statement locations (can be merged with the one above)
- Don't use `foo` or `bar` custom names, always use `test_{object}`, e.g. `SELECT * FROM test_table`, not `SELECT * FROM hehe_haha`. If you need multiple names, use `_{number}` suffix, e.g. `SELECT test_field, test_field_2 FROM test_table;`
- Write all the static tokens in UPPER_CASE, and all the custom variables in lower_case, e.g. `SELECT test_field`
- Always test your statements on errors, and if there's an unexpected error, just add `TODO: fix unhandled error` error
33 changes: 21 additions & 12 deletions src/parsing/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,23 +9,17 @@ export abstract class Parser {
}

export interface ParseResult {
locations: StatementPart[];
locations: StatementPart[]; // TODO: figure our if it's optional
errors?: SyntaxError[];
suggestKeywords?: KeywordSuggestion[];
suggestTables?: {
prependFrom?: boolean;
prependQuestionMark?: boolean;
onlyTables?: boolean;
};
suggestTables?: TablesSuggestion;
suggestColumns?: ColumnSuggestion;
suggestAggregateFunctions?: {
tables: Table[],
};
suggestAggregateFunctions?: AggregateFunctionsSuggestion;
suggestAnalyticFunctions?: unknown;
suggestColRefKeywords?: unknown;
suggestColumnAliases?: ColumnAliasSuggestion[];
suggestCommonTableExpressions?: unknown;
suggestDatabases?: unknown;
suggestDatabases?: DatabasesSuggestion;
suggestFilters?: unknown;
suggestFunctions?: unknown;
suggestGroupBys?: unknown;
Expand All @@ -37,7 +31,7 @@ export interface ParseResult {
};

// Reasons for those fields are unknown
definitions?: [];
definitions?: []; // TODO: figure our if it's optional
lowerCase: boolean;
}

Expand Down Expand Up @@ -87,8 +81,23 @@ export type StatementPart =
qualified: boolean,
};

export interface TablesSuggestion {
prependFrom?: boolean; // TODO: figure our if it's optional
prependQuestionMark?: boolean; // TODO: figure our if it's optional
onlyTables?: boolean; // TODO: figure our if it's optional
onlyViews?: boolean; // TODO: figure our if it's optional
}

export interface DatabasesSuggestion {
appendDot?: boolean; // TODO: figure our if it's optional
}

export interface AggregateFunctionsSuggestion {
tables: Table[],
}

export interface ColumnSuggestion {
source?: string;
source?: string; // TODO: figure our if it's optional
tables: Table[];
}

Expand Down
2 changes: 1 addition & 1 deletion src/parsing/parsers/clickhouse/jison/structure.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"../../generic/jison/alter/alter_table.jison",
"../../generic/jison/alter/alter_view.jison",
"../../generic/jison/create/create_common.jison",
"../../generic/jison/create/create_database.jison",
"../../generic/jison/create/create_database_or_schema.jison",
"../../generic/jison/create/create_role.jison",
"create/create_table.jison",
"../../generic/jison/create/create_view.jison",
Expand Down
20 changes: 0 additions & 20 deletions src/parsing/parsers/generic/jison/alter/alter_common.test.json

This file was deleted.

14 changes: 14 additions & 0 deletions src/parsing/parsers/generic/jison/alter/alter_common.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import {
KeywordSuggestion,
parseGenericSql,
} from '../../../../index';
import {expect, test} from '@jest/globals';

test('should suggest ALTER', () => {
const parseResult = parseGenericSql('', '');

expect(parseResult.errors).toBeUndefined();

const suggestion: KeywordSuggestion = { value: 'ALTER', weight: -1 };
expect(parseResult.suggestKeywords).toContainEqual(suggestion);
})
1 change: 1 addition & 0 deletions src/parsing/parsers/generic/jison/alter/alter_table.jison
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ AlterTable
: AlterTableLeftSide PartitionSpec
;

// TODO: support AlterTableRightSide, PartitionSpec will not work for MySQL, PostgreSQL
AlterTable_EDIT
: AlterTableLeftSide_EDIT
| AlterTableLeftSide_EDIT PartitionSpec
Expand Down
41 changes: 0 additions & 41 deletions src/parsing/parsers/generic/jison/alter/alter_table.test.json

This file was deleted.

33 changes: 33 additions & 0 deletions src/parsing/parsers/generic/jison/alter/alter_table.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import {
DatabasesSuggestion,
KeywordSuggestion,
parseGenericSql, TablesSuggestion,
} from '../../../../index';
import {expect, test} from '@jest/globals';

test('should suggest altering table', () => {
const parseResult = parseGenericSql('ALTER ', '');

expect(parseResult.errors).toBeUndefined();

const suggestion: KeywordSuggestion = { value: 'TABLE', weight: -1 };
expect(parseResult.suggestKeywords).toContainEqual(suggestion);
})

test('should suggest tables to alter', () => {
const parseResult = parseGenericSql('ALTER TABLE ', '');

expect(parseResult.errors).toBeUndefined();

const tablesSuggestion: TablesSuggestion = {
onlyTables: true,
};
expect(parseResult.suggestTables).toEqual(tablesSuggestion);

const databasesSuggestion: DatabasesSuggestion = {
appendDot: true,
}
expect(parseResult.suggestDatabases).toEqual(databasesSuggestion);
})

// TODO: add full tests + locations test
81 changes: 0 additions & 81 deletions src/parsing/parsers/generic/jison/alter/alter_view.test.json

This file was deleted.

Loading

0 comments on commit 08e8c26

Please sign in to comment.