Skip to content

Commit 3c7c59f

Browse files
authored
feat: optimize suggestion (#231)
* feat: optimize the strategy of finding the right range * test: apply commentOtherLine util to all suggestion tests * test: decomment suggestion test cases * test: add suggestion test cases in multiple statements * chore: improve comments * test: update log info in test
1 parent fd50c09 commit 3c7c59f

29 files changed

+934
-159
lines changed

src/parser/common/basicParser.ts

Lines changed: 60 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -277,55 +277,74 @@ export default abstract class BasicParser<
277277

278278
/**
279279
* Split sql by statement.
280-
* Try to collect candidates from the caret statement only.
280+
* Try to collect candidates in as small a range as possible.
281281
*/
282282
this.listen(splitListener, this._parseTree);
283+
const statementCount = splitListener.statementsContext?.length;
284+
const statementsContext = splitListener.statementsContext;
283285

284286
// If there are multiple statements.
285-
if (splitListener.statementsContext.length > 1) {
286-
// find statement rule context where caretPosition is located.
287-
const caretStatementContext = splitListener?.statementsContext.find((ctx) => {
288-
return (
289-
caretTokenIndex <= ctx.stop?.tokenIndex &&
290-
caretTokenIndex >= ctx.start.tokenIndex
291-
);
292-
});
293-
294-
if (caretStatementContext) {
295-
c3Context = caretStatementContext;
296-
} else {
297-
const lastStatementToken =
298-
splitListener.statementsContext[splitListener?.statementsContext.length - 1]
299-
.start;
287+
if (statementCount > 1) {
288+
/**
289+
* Find a minimum valid range, reparse the fragment, and provide a new parse tree to C3.
290+
* The boundaries of this range must be statements with no syntax errors.
291+
* This can ensure the stable performance of the C3.
292+
*/
293+
let startStatement: ParserRuleContext;
294+
let stopStatement: ParserRuleContext;
295+
296+
for (let index = 0; index < statementCount; index++) {
297+
const ctx = statementsContext[index];
298+
const isCurrentCtxValid = !ctx.exception;
299+
if (!isCurrentCtxValid) continue;
300+
300301
/**
301-
* If caretStatementContext is not found and it follows all statements.
302-
* Reparses part of the input following the penultimate statement.
303-
* And c3 will collect candidates in the new parseTreeContext.
302+
* Ensure that the statementContext before the left boundary
303+
* and the last statementContext on the right boundary are qualified SQL statements.
304304
*/
305-
if (caretTokenIndex > lastStatementToken?.tokenIndex) {
306-
/**
307-
* Save offset of the tokenIndex in the partInput
308-
* compared to the tokenIndex in the whole input
309-
*/
310-
tokenIndexOffset = lastStatementToken?.tokenIndex;
311-
// Correct caretTokenIndex
312-
caretTokenIndex = caretTokenIndex - tokenIndexOffset;
313-
314-
const inputSlice = input.slice(lastStatementToken.startIndex);
315-
const lexer = this.createLexer(inputSlice);
316-
lexer.removeErrorListeners();
317-
318-
const tokenStream = new CommonTokenStream(lexer);
319-
tokenStream.fill();
320-
const parser = this.createParserFromTokenStream(tokenStream);
321-
parser.removeErrorListeners();
322-
parser.buildParseTree = true;
323-
parser.errorHandler = new ErrorStrategy();
324-
325-
sqlParserIns = parser;
326-
c3Context = parser.program();
305+
const isPrevCtxValid = index === 0 || !statementsContext[index - 1]?.exception;
306+
const isNextCtxValid =
307+
index === statementCount - 1 || !statementsContext[index + 1]?.exception;
308+
309+
if (ctx.stop.tokenIndex < caretTokenIndex && isPrevCtxValid) {
310+
startStatement = ctx;
311+
}
312+
313+
if (!stopStatement && ctx.start.tokenIndex > caretTokenIndex && isNextCtxValid) {
314+
stopStatement = ctx;
315+
break;
327316
}
328317
}
318+
319+
// A boundary consisting of the index of the input.
320+
const startIndex = startStatement?.start?.startIndex ?? 0;
321+
const stopIndex = stopStatement?.stop?.stopIndex ?? input.length - 1;
322+
323+
/**
324+
* Save offset of the tokenIndex in the range of input
325+
* compared to the tokenIndex in the whole input
326+
*/
327+
tokenIndexOffset = startStatement?.start?.tokenIndex ?? 0;
328+
caretTokenIndex = caretTokenIndex - tokenIndexOffset;
329+
330+
/**
331+
* Reparse the input fragment,
332+
* and c3 will collect candidates in the newly generated parseTree.
333+
*/
334+
const inputSlice = input.slice(startIndex, stopIndex);
335+
336+
const lexer = this.createLexer(inputSlice);
337+
lexer.removeErrorListeners();
338+
const tokenStream = new CommonTokenStream(lexer);
339+
tokenStream.fill();
340+
341+
const parser = this.createParserFromTokenStream(tokenStream);
342+
parser.removeErrorListeners();
343+
parser.buildParseTree = true;
344+
parser.errorHandler = new ErrorStrategy();
345+
346+
sqlParserIns = parser;
347+
c3Context = parser.program();
329348
}
330349

331350
const core = new CodeCompletionCore(sqlParserIns);

test/parser/flinksql/suggestion/fixtures/multipleSql.sql

Lines changed: 0 additions & 19 deletions
This file was deleted.
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
SELECT * FROM -- unfinished
2+
3+
CREATE TEMPORARY VIEW IF NOT EXISTS v AS SELECT col1 FROM tbl;
4+
5+
CREATE TEMPORARY TABLE client_errors (
6+
log_time TIMESTAMP(3),
7+
request_line STRING,
8+
status_code STRING,
9+
size INT
10+
) WITH (
11+
'connector' = 'stream-x'
12+
);
13+
14+
ALTER VIEW v1 RENAME TO v2;
15+
16+
CREATE TABLE db. ; -- unfinished
17+
18+
LOAD MODULE CORE;
19+
20+
REMOVE JAR '<path_to_filename>.jar'
21+
22+
INSERT INTO VALUES (100, 99.9 / 10, 'abc', true, now ()); -- unfinished
23+
24+
CREATE DATABASE IF NOT EXISTS dataApi COMMENT 'test create database' WITH ('key1' = 'value1', 'key2.a' = 'value2.a');
25+
26+
DROP DATABASE IF EXISTS Orders RESTRICT;
27+
28+
INSERT INTO country_page_view
29+
SELECT `user`,
30+
cnt
31+
FROM db. ; -- unfinished
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
import fs from 'fs';
2+
import path from 'path';
3+
import { CaretPosition, SyntaxContextType } from '../../../../src/parser/common/basic-parser-types';
4+
import FlinkSQL from '../../../../src/parser/flinksql';
5+
6+
const syntaxSql = fs.readFileSync(
7+
path.join(__dirname, 'fixtures', 'multipleStatement.sql'),
8+
'utf-8'
9+
);
10+
11+
describe('FlinkSQL Multiple Statements Syntax Suggestion', () => {
12+
const parser = new FlinkSQL();
13+
14+
test('Select from table ', () => {
15+
const pos: CaretPosition = {
16+
lineNumber: 1,
17+
column: 15,
18+
};
19+
const syntaxes = parser.getSuggestionAtCaretPosition(syntaxSql, pos)?.syntax;
20+
const suggestion = syntaxes?.find(
21+
(syn) => syn.syntaxContextType === SyntaxContextType.TABLE
22+
);
23+
24+
expect(suggestion).not.toBeUndefined();
25+
expect(suggestion?.wordRanges.map((token) => token.text)).toEqual([]);
26+
});
27+
28+
test('Create table ', () => {
29+
const pos: CaretPosition = {
30+
lineNumber: 16,
31+
column: 17,
32+
};
33+
const syntaxes = parser.getSuggestionAtCaretPosition(syntaxSql, pos)?.syntax;
34+
const suggestion = syntaxes?.find(
35+
(syn) => syn.syntaxContextType === SyntaxContextType.TABLE_CREATE
36+
);
37+
38+
expect(suggestion).not.toBeUndefined();
39+
expect(suggestion?.wordRanges.map((token) => token.text)).toEqual(['db', '.']);
40+
});
41+
42+
test('Insert into table ', () => {
43+
const pos: CaretPosition = {
44+
lineNumber: 22,
45+
column: 13,
46+
};
47+
const syntaxes = parser.getSuggestionAtCaretPosition(syntaxSql, pos)?.syntax;
48+
const suggestion = syntaxes?.find(
49+
(syn) => syn.syntaxContextType === SyntaxContextType.TABLE
50+
);
51+
52+
expect(suggestion).not.toBeUndefined();
53+
expect(suggestion?.wordRanges.map((token) => token.text)).toEqual([]);
54+
});
55+
56+
test('Insert into select from table ', () => {
57+
const pos: CaretPosition = {
58+
lineNumber: 31,
59+
column: 9,
60+
};
61+
const syntaxes = parser.getSuggestionAtCaretPosition(syntaxSql, pos)?.syntax;
62+
const suggestion = syntaxes?.find(
63+
(syn) => syn.syntaxContextType === SyntaxContextType.TABLE
64+
);
65+
66+
expect(suggestion).not.toBeUndefined();
67+
expect(suggestion?.wordRanges.map((token) => token.text)).toEqual(['db', '.']);
68+
});
69+
});

test/parser/flinksql/suggestion/syntaxSuggestion.test.ts

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ const syntaxSql = fs.readFileSync(
88
path.join(__dirname, 'fixtures', 'syntaxSuggestion.sql'),
99
'utf-8'
1010
);
11-
const multipleSql = fs.readFileSync(path.join(__dirname, 'fixtures', 'multipleSql.sql'), 'utf-8');
1211

1312
describe('Flink SQL Syntax Suggestion', () => {
1413
const parser = new FlinkSQL();
@@ -19,20 +18,6 @@ describe('Flink SQL Syntax Suggestion', () => {
1918
expect(parser.validate(syntaxSql).length).not.toBe(0);
2019
});
2120

22-
test('Multiple SQL use database', () => {
23-
const pos: CaretPosition = {
24-
lineNumber: 19,
25-
column: 10,
26-
};
27-
const syntaxes = parser.getSuggestionAtCaretPosition(multipleSql, pos)?.syntax;
28-
const suggestion = syntaxes?.find(
29-
(syn) => syn.syntaxContextType === SyntaxContextType.DATABASE
30-
);
31-
32-
expect(suggestion).not.toBeUndefined();
33-
expect(suggestion?.wordRanges.map((token) => token.text)).toEqual(['cat1', '.']);
34-
});
35-
3621
test('Drop catalog', () => {
3722
const pos: CaretPosition = {
3823
lineNumber: 1,

test/parser/flinksql/suggestion/tokenSuggestion.test.ts

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import fs from 'fs';
22
import path from 'path';
33
import { CaretPosition } from '../../../../src/parser/common/basic-parser-types';
44
import FlinkSQL from '../../../../src/parser/flinksql';
5+
import { commentOtherLine } from '../../../helper';
56

67
const tokenSql = fs.readFileSync(path.join(__dirname, 'fixtures', 'tokenSuggestion.sql'), 'utf-8');
78

@@ -13,7 +14,10 @@ describe('Flink SQL Token Suggestion', () => {
1314
lineNumber: 3,
1415
column: 5,
1516
};
16-
const suggestion = parser.getSuggestionAtCaretPosition(tokenSql, pos)?.keywords;
17+
const suggestion = parser.getSuggestionAtCaretPosition(
18+
commentOtherLine(tokenSql, pos.lineNumber),
19+
pos
20+
)?.keywords;
1721

1822
expect(suggestion).toEqual(['MODULES', 'CATALOG']);
1923
});
@@ -23,7 +27,10 @@ describe('Flink SQL Token Suggestion', () => {
2327
lineNumber: 5,
2428
column: 8,
2529
};
26-
const suggestion = parser.getSuggestionAtCaretPosition(tokenSql, pos)?.keywords;
30+
const suggestion = parser.getSuggestionAtCaretPosition(
31+
commentOtherLine(tokenSql, pos.lineNumber),
32+
pos
33+
)?.keywords;
2734

2835
expect(suggestion).toEqual([
2936
'CATALOG',
@@ -40,7 +47,10 @@ describe('Flink SQL Token Suggestion', () => {
4047
lineNumber: 7,
4148
column: 6,
4249
};
43-
const suggestion = parser.getSuggestionAtCaretPosition(tokenSql, pos)?.keywords;
50+
const suggestion = parser.getSuggestionAtCaretPosition(
51+
commentOtherLine(tokenSql, pos.lineNumber),
52+
pos
53+
)?.keywords;
4454

4555
expect(suggestion).toEqual([
4656
'MODULES',
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
SELECT * FROM -- unfinished
2+
3+
CREATE VIEW mydb.bro_view AS SELECT * FROM mydb.sale_tbl;
4+
5+
CREATE TEMPORARY EXTERNAL TABLE list_bucket_multiple (col1 STRING, col2 INT, col3 STRING);
6+
7+
ALTER VIEW myview1 SET TBLPROPERTIES ('author'='hayden','date'='2023-09-04')
8+
9+
CREATE TABLE db. ; -- unfinished
10+
11+
DROP CONNECTOR connector1;
12+
13+
SET ROLE `admin`;
14+
15+
INSERT INTO VALUES (100, 99.9 / 10, 'abc', true, now ()); -- unfinished
16+
17+
ALTER TABLE tbl1 RENAME TO tbl2;
18+
19+
ALTER SCHEMA database_name SET OWNER USER `admin`;
20+
21+
INSERT OVERWRITE LOCAL DIRECTORY '/path/to/output' SELECT col1, col2 FROM ; -- unfinished

0 commit comments

Comments
 (0)