Skip to content

update SingleSelectOperator - #903

Open
SkywalkerSpace wants to merge 1 commit into
masterfrom
update-SingleSelectOperator
Open

update SingleSelectOperator#903
SkywalkerSpace wants to merge 1 commit into
masterfrom
update-SingleSelectOperator

Conversation

@SkywalkerSpace

Copy link
Copy Markdown
Contributor

No description provided.

@seafile-dev seafile-dev left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

结论:Comment

Warnings:

  • 单选值包含单引号的 IS 条件已改为调用统一转义逻辑;但本 PR 未加入回归测试(见行内评论)。
    Suggested fix: 为 SingleSelectOperator.op_is 增加包含单个和多个单引号的断言,并验证生成的 SQL 使用 ''

self.column_name,
'=',
filter_term
self.escape_string(filter_term)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[Warning] 缺少单引号回归测试

Why this matters:
本次修复专门处理选项名称中的单引号,但 PR 没有覆盖该场景;以后调整 op_is 或转义规则时,French a'c 仍可能再次生成无法解析的 SQL 并在图表中变成 500。

Suggested fix: 为 SingleSelectOperator.op_is 增加含一个及多个单引号的测试,断言 SQL 字面量使用 '',例如 French a''c

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