Skip to content

Conversation

@winfredLIN
Copy link
Collaborator

@winfredLIN winfredLIN commented Oct 23, 2025

User description

关联的 issue

#3049

描述你的变更

  1. 修复:V2规则:禁止WHERE子句中条件字段与值的数据类型不一致,误触发的问题

确认项(pr提交后操作)

Tip

请在指定复审人之前,确认并完成以下事项,完成后✅


  • 我已完成自测
  • 我已记录完整日志方便进行诊断
  • 我已在关联的issue里补充了实现方案
  • 我已在关联的issue里补充了测试影响面
  • 我已确认了变更的兼容性,如果不兼容则在issue里标记 not_compatible
  • 我已确认了是否要更新文档,如果要更新则在issue里标记 need_update_doc


Description

  • 修复WHERE子句中类型判断错误

  • 增加多函数校验整数、字符串、BLOB兼容性

  • 重构getExprType为getExprTypeInfo返回UNSIGNED信息

  • 增加大量测试用例覆盖隐式转换及边界情况


Diagram Walkthrough

flowchart LR
  A["获取表达式类型"] --> B["调用getExprTypeInfo"]
  B --> C["判断类型兼容关系"]
  C --> D["调用isTypeCompatible"]
  D --> E["报告规则结果"]
Loading

File Walkthrough

Relevant files
Enhancement
rule_00112.go
重构规则函数及增强类型判断                                                                                       

sqle/driver/mysql/rule/ai/rule_00112.go

  • 新增辅助函数: getValueFromExpr, isIntegerValueInRange等
  • 引入getExprTypeInfo返回类型和UNSIGNED信息
  • 重构isTypeCompatible及兼容性校验逻辑
  • 调整规则逻辑以避免隐式类型转换问题
+344/-54
Tests
rule_00112_test.go
新增覆盖各场景的规则测试                                                                                         

sqle/driver/mysql/rule_00112_test.go

  • 增加针对多种类型场景的新测试用例
  • 涵盖整数、字符串、日期、BLOB及边界情况测试
  • 测试案例编号从33至75
+278/-0 

linxiaotao added 2 commits October 23, 2025 05:45
- Introduced functions to assess compatibility between various data types (integers, strings, BLOBs, and DECIMALs).
- Updated the main rule logic to utilize these new checks, ensuring accurate validation of WHERE/ON/USING clause conditions.
- Added comprehensive comments detailing the logic and scenarios for type compatibility, addressing limitations of the current SQL parser.

This enhancement aims to prevent performance issues caused by implicit type conversions in SQL queries.
…SQLE00112

- Introduced new test cases to validate behavior of various data types (e.g., BIGINT, INT, FLOAT, CHAR, DATE) in SQL queries.
- Enhanced coverage for implicit conversion scenarios, ensuring accurate detection of type mismatches.
- Organized tests into categories for better clarity and maintainability, addressing edge cases and boundary conditions.

These additions aim to strengthen the robustness of type compatibility checks in SQL queries.
@github-actions
Copy link

github-actions bot commented Oct 23, 2025

PR Reviewer Guide 🔍

(Review updated until commit 2bbf896)

🎫 Ticket compliance analysis ✅

3049 - Fully compliant

Compliant requirements:

  • 修复 WHERE 子句中条件字段与值数据类型不一致误报问题
  • 增加多函数校验及重构 getExprTypeInfo 函数
  • 补充全面的测试用例覆盖各类场景
⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

错误处理

在 getExprTypeInfo 函数中,当遍历 CREATE TABLE 的列定义未能找到匹配的列时,返回的错误信息较为简略。建议提供更多上下文信息(例如表名或者该列所处的 SQL 环境),以便后续调试和问题定位。

	createTableStmt, err := util.GetCreateTableStmt(input.Ctx, &ast.TableName{Name: model.NewCIStr(tableName)})
	if err != nil {
		return 0, false, err
	}
	// 获取列定义
	for _, colDef := range createTableStmt.Cols {
		if strings.EqualFold(util.GetColumnName(colDef), node.Name.Name.O) {
			isUnsigned := mysql.HasUnsignedFlag(colDef.Tp.Flag)
			return colDef.Tp.Tp, isUnsigned, nil
		}
	}
	return 0, false, fmt.Errorf("列未找到: %s", node.Name.Name.O)
case *ast.FuncCallExpr:
魔法数字

在 isIntegerValueInRange 和 isIntegerValueInUnsignedRange 等函数中,直接硬编码了多个数值常量。建议将这些边界值定义为具名常量,以增强代码的可读性与可维护性。

// isIntegerValueInRange 检查整数值是否在目标类型范围内
func isIntegerValueInRange(value int64, targetType byte) bool {
	switch targetType {
	case mysql.TypeTiny:
		return value >= -128 && value <= 127
	case mysql.TypeShort:
		return value >= -32768 && value <= 32767
	case mysql.TypeInt24:
		return value >= -8388608 && value <= 8388607
	case mysql.TypeLong:
		return value >= -2147483648 && value <= 2147483647
	case mysql.TypeLonglong:
		return true
	}
	return false

@github-actions
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
添加 nil 检查

建议在 getValueFromExpr 函数开始处增加对 exprexpr.Datum 的 nil 检查,防止在访问时产生 nil
指针异常。这样可以避免在遇到异常输入时触发 panic。

sqle/driver/mysql/rule/ai/rule_00112.go [145-146]

 func getValueFromExpr(expr *parserdriver.ValueExpr) (int64, bool) {
+	if expr == nil || expr.Datum == nil {
+		return 0, false
+	}
 	switch expr.Datum.Kind() {
Suggestion importance[1-10]: 6

__

Why: 该建议在 getValueFromExpr 函数中增加对 expr 及其 Datum 的 nil 检查,提升了代码健壮性以避免潜在的 nil 指针异常,但仅为小幅的安全性改进,因此得分为6。

Low

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


linxiaotao seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@github-actions
Copy link

Persistent review updated to latest commit 2bbf896

@github-actions
Copy link

PR Code Suggestions ✨

No code suggestions found for the PR.

@iwanghc iwanghc merged commit d1804f1 into main Oct 24, 2025
3 of 4 checks passed
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.

4 participants