Skip to content

fix: #436 improve SQL keyword suggestions - #485

Open
liuxy0551 wants to merge 2 commits into
DTStack:nextfrom
liuxy0551:fix_436
Open

fix: #436 improve SQL keyword suggestions#485
liuxy0551 wants to merge 2 commits into
DTStack:nextfrom
liuxy0551:fix_436

Conversation

@liuxy0551

@liuxy0551 liuxy0551 commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

本次改动

Fixes #436 优化光标位置的 SQL 关键字补全:

  • 默认只保留当前 SQL 语句的关键字候选
  • 避免未结束语句后出现 SELECT 等下一语句起始关键字
  • 保留当前语句合法的续写关键字和组合关键字
  • 新增可选的 keywordFilter 回调,支持调用方自定义过滤
  • 关键字过滤不影响语法补全结果

API 使用方式

const excludedKeywords = new Set(['WHERE', 'ORDER BY']);

const suggestions = parser.getSuggestionAtCaretPosition(sql, position, {
    keywordFilter: (keyword) => !excludedKeywords.has(keyword),
});

keywordFilter 针对最终生成的关键字执行:

  • 返回 true:保留关键字
  • 返回 false:移除关键字

效果展示

https://liuxy0551.github.io/monaco-sql-languages/

当前语句关键字补全

image

补全列表只包含当前查询合法的续写关键字,不再出现 SELECT 等下一语句起始关键字。

输入部分关键字后的补全

image

输入 s 后,只展示 SORTSORT BY 等符合当前位置的候选项。

测试

  • 覆盖 MySQL、FlinkSQL、SparkSQL、HiveSQL、PostgreSQL、TrinoSQL、ImpalaSQL、GenericSQL
  • 覆盖无分号、分号后、多语句和未完成单词等场景
  • 覆盖黑名单、白名单和组合关键字过滤
  • 验证过滤逻辑不影响语法补全
  • 全量测试及 TypeScript 类型检查通过

@liuxy0551
liuxy0551 requested a review from Cythia828 July 28, 2026 09:30
@Cythia828

Copy link
Copy Markdown
Collaborator

补全列表只包含当前查询合法的续写关键字,不再出现 SELECT 等下一语句起始关键字。这一改动没问题,但是用户如果开始一句新的SQL了,仍然没有关键字提示会不会不友好?比如我已经加了;,换行开始新的SQL,但没有任何提示

Comment on lines +699 to +703
const candidates = this.collectSuggestionCandidates(
sqlParserIns,
parseTree,
caretTokenIndex
);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

collectSuggestionCandidates 在最坏情况下会跑 3 次 collectCandidates(整棵树 + 语句树 + 仅在首次缓存的语句起始 token 集)。常见未切片场景是 2 次。对大 SQL 输入这是可感知的额外开销。建议补充一个针对大输入的 benchmark,确认回归在可接受范围。

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

三次 collectCandidates 是「两层隔离」保证正确性的刻意设计——Issue 436 之前的单次 C3 正是把下一条语句的 SELECT/CREATE/INSERT 混进当前补全的根源。三步各司其职:

  1. program —— 保留外层 rule path、syntax 候选与上下文;
  2. statement 子树 —— 过滤下一条语句的起始关键字,保留 LOCK IN SHARE MODE 等组合关键字;
  3. token 0(仅首次,缓存在 _statementStartTokenTypes —— 得到语句起始关键字集合,只删 SELECT/CREATE/INSERT 这类起始词。

调用次数:首次最多 3 次,后续 2 次,分号后或无需隔离 1 次。

不能砍掉 statement 那次,否则退化回 Issue 436 之前的错误(复核确认会丢 MySQL LOCK IN SHARE MODE、Impala WITH SERDEPROPERTIES,嵌套查询也会重新混入顶层起始关键字)。

性能 A/B 对比(热启动 50 次取中位数):

场景 修复前 修复后
SELECT ... WHERE ... (无分号末尾) 0.78 ms / 93 关键词 1.75 ms / 28 关键词
60 行 SELECT 的 FROM 之后 3.6 ms 5.8 ms

额外成本就是一次等量 statement C3,毫秒级,误报的 SELECT/INSERT/CREATE 全部消失(93→28),可接受。

benchmark 方面:benchmark:release 冷启动的 clearATNCache() 存在预先存在的内存泄漏(每次 +30MB 无法 GC,累积 OOM),与本次改动无关,故改用手工测量;OOM 我会另开 issue 跟进。

结论:接受首次 3 次、后续 2 次的实现。

const children = parseTree.children;
if (!children?.length) return parseTree;

for (let index = children.length - 1; index >= 0; index--) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

你这里是只向下看一层,它假设 program 的直接子节点就是语句。一旦文法把语句包了一层中间规则(如 program → batch → statement,或 list 规则),收窄会静默回退到整棵树,修复悄悄失效且无报错,确认下这个链路在这种情况下是否有问题?是否需要加一条断言/测试守护该假设?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

已经加了守护测试:suggestion.test.ts 的用例遍历全部方言,断言 SELECT * FROM t; SELECT * FROM u 解析出的 program 直接子节点恰好是两个 statement。

所有方言 grammar 都是 program: (statement SEMI?)* 扁平结构,直接子节点即语句;只向下看一层是刻意的,避免误选嵌套语句/子查询(见 getSuggestionParseTree 上方注释)。

若未来引入中间规则,这条测试会立即失败暴露,不会静默回退。

@liuxy0551

Copy link
Copy Markdown
Collaborator Author

补全列表只包含当前查询合法的续写关键字,不再出现 SELECT 等下一语句起始关键字。这一改动没问题,但是用户如果开始一句新的SQL了,仍然没有关键字提示会不会不友好?比如我已经加了;,换行开始新的SQL,但没有任何提示

换行不会触发补全提示,空格会触发。测试了下,修改后按空格依旧是可以出发补全提示的。

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.

2 participants