Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

当子条件的查询表达式为空时,构建sql拼接了多余的LeftBracket和RightBracket #74

Open
PhoenixL0911 opened this issue Sep 15, 2023 · 6 comments
Labels
bug Something isn't working

Comments

@PhoenixL0911
Copy link
Contributor

bug案例复现:

func TestSelectListOr(t *testing.T) {
	var expectSql = "SELECT * FROM `Users` WHERE username = 'afumu' OR age = 20"
	sessionDb := checkSelectSql(t, expectSql)
	query, u := gplus.NewQuery[User]()
	query.Eq(&u.Username, "afumu").Or().Eq(&u.Age, 20).Or(func(q *gplus.QueryCond[User]) {

	})
	gplus.SelectList[User](query, gplus.Db(sessionDb))
}

期望得到的结果:

SELECT * FROM `Users` WHERE username = 'afumu' OR age = 20

实际得到的结果:

SELECT * FROM `Users` WHERE username = 'afumu' OR age OR () = 20

导致bug出现bug的函数:

func buildSqlAndArgs[T any](expressions []any, sqlBuilder *strings.Builder, queryArgs []any) []any {
	for _, v := range expressions {
		// 判断是否是columnValue类型
		switch segment := v.(type) {
		case *columnPointer:
			sqlBuilder.WriteString(segment.getSqlSegment() + " ")
		case *sqlKeyword:
			sqlBuilder.WriteString(segment.getSqlSegment() + " ")
		case *columnValue:
			if segment.value == constants.And {
				sqlBuilder.WriteString(segment.value.(string) + " ")
				continue
			}
			if segment.value != "" {
				sqlBuilder.WriteString("? ")
				queryArgs = append(queryArgs, segment.value)
			}
		case *QueryCond[T]:
			sqlBuilder.WriteString(constants.LeftBracket + " ")
			// 递归处理条件
			queryArgs = buildSqlAndArgs[T](segment.queryExpressions, sqlBuilder, queryArgs)
			sqlBuilder.WriteString(constants.RightBracket + " ")
		}
	}
	return queryArgs
}

bug fix方案:

func buildSqlAndArgs[T any](expressions []any, sqlBuilder *strings.Builder, queryArgs []any) []any {
	for _, v := range expressions {
		// 判断是否是columnValue类型
		switch segment := v.(type) {
		case *columnPointer:
			sqlBuilder.WriteString(segment.getSqlSegment() + " ")
		case *sqlKeyword:
			sqlBuilder.WriteString(segment.getSqlSegment() + " ")
		case *columnValue:
			if segment.value == constants.And {
				sqlBuilder.WriteString(segment.value.(string) + " ")
				continue
			}
			if segment.value != "" {
				sqlBuilder.WriteString("? ")
				queryArgs = append(queryArgs, segment.value)
			}
		case *QueryCond[T]:
			// 当子条件不存在查询表达式时,无需进行递归处理
			if len(segment.queryExpressions) == 0 {
				continue
			}
			sqlBuilder.WriteString(constants.LeftBracket + " ")
			// 递归处理条件
			queryArgs = buildSqlAndArgs[T](segment.queryExpressions, sqlBuilder, queryArgs)
			sqlBuilder.WriteString(constants.RightBracket + " ")
		}
	}
	return queryArgs
}
@aixj1984
Copy link
Contributor

提交的解决方案还有问题:

func TestSelectListOr2(t *testing.T) {
var expectSql = "SELECT * FROM Users WHERE username = 'afumu' OR age = 20"
sessionDb := checkSelectSql(t, expectSql)
query, u := gplus.NewQueryUser
query.Eq(&u.Username, "afumu").Or().Eq(&u.Age, 20).Or(func(q *gplus.QueryCond[User]) {

})
gplus.SelectList[User](query, gplus.Db(sessionDb))

}

2023/09/15 11:55:12 D:/test/gorm-plus/gplus/dao.go:197
[0.000ms] [rows:0] SELECT * FROM Users WHERE username = 'afumu' OR age = 20 OR
--- FAIL: TestSelectListOr2 (0.00s)
select_test.go:312: errors happened when select expect: SELECT * FROM Users WHERE username = 'afumu' OR age = 20, got SELECT * FROM Users WHERE username = 'afumu' OR age = 20 OR

@PhoenixL0911
Copy link
Contributor Author

PhoenixL0911 commented Sep 15, 2023

提交的解决方案还有问题:

func TestSelectListOr2(t *testing.T) { var expectSql = "SELECT * FROM Users WHERE username = 'afumu' OR age = 20" sessionDb := checkSelectSql(t, expectSql) query, u := gplus.NewQueryUser query.Eq(&u.Username, "afumu").Or().Eq(&u.Age, 20).Or(func(q *gplus.QueryCond[User]) {

})
gplus.SelectList[User](query, gplus.Db(sessionDb))

}

2023/09/15 11:55:12 D:/test/gorm-plus/gplus/dao.go:197 [0.000ms] [rows:0] SELECT * FROM Users WHERE username = 'afumu' OR age = 20 OR --- FAIL: TestSelectListOr2 (0.00s) select_test.go:312: errors happened when select expect: SELECT * FROM Users WHERE username = 'afumu' OR age = 20, got SELECT * FROM Users WHERE username = 'afumu' OR age = 20 OR

最直接的方法应该是从Or()和And()这两个方法去做限制,例如:

// And 拼接 AND
func (q *QueryCond[T]) And(fn ...func(q *QueryCond[T])) *QueryCond[T] {
	q.addExpression(&sqlKeyword{keyword: constants.And})
	if len(fn) > 0 {
		nestQuery := &QueryCond[T]{}
		fn[0](nestQuery)
		// 如果 nestQuery 不存在查询表达式,则放弃该条件,不添加到条件组中
		if len(nestQuery.queryExpressions) == 0 {
			return q
		}
		q.queryExpressions = append(q.queryExpressions, nestQuery)
		q.last = nestQuery
		return q
	}
	return q
}

// Or 拼接 OR
func (q *QueryCond[T]) Or(fn ...func(q *QueryCond[T])) *QueryCond[T] {
	q.addExpression(&sqlKeyword{keyword: constants.Or})
	if len(fn) > 0 {
		nestQuery := &QueryCond[T]{}
		fn[0](nestQuery)
		// 如果 nestQuery 不存在查询表达式,则放弃该条件,不添加到条件组中
		if len(nestQuery.queryExpressions) > 0 {
			return q
		}
		q.queryExpressions = append(q.queryExpressions, nestQuery)
		q.last = nestQuery
		return q
	}
	return q
}

但是如果以后存在构建无条件子查询功能的需求,最后还是得回到buildSqlAndArgs()这个函数去解决。

@afumu afumu added the bug Something isn't working label Sep 15, 2023
@aixj1984
Copy link
Contributor

最新的代码,修改还不对
2023/09/15 18:06:23 D:/test/gorm-plus/gplus/dao.go:197
[0.541ms] [rows:0] SELECT * FROM Users WHERE username = 'afumu' OR age = 20 OR
--- FAIL: TestSelectListOr2 (0.00s)

@afumu
Copy link
Member

afumu commented Sep 15, 2023

@DeerSunny 确实有问题,可以这么判断

image

最新的代码,修改还不对 2023/09/15 18:06:23 D:/test/gorm-plus/gplus/dao.go:197 [0.541ms] [rows:0] SELECT * FROM Users WHERE username = 'afumu' OR age = 20 OR --- FAIL: TestSelectListOr2 (0.00s)

@PhoenixL0911
Copy link
Contributor Author

PhoenixL0911 commented Sep 16, 2023

@DeerSunny 确实有问题,可以这么判断

image

最新的代码,修改还不对 2023/09/15 18:06:23 D:/test/gorm-plus/gplus/dao.go:197 [0.541ms] [rows:0] SELECT * FROM Users WHERE username = 'afumu' OR age = 20 OR --- FAIL: TestSelectListOr2 (0.00s)

这方案我上面已经提到过了其实,当时不想这么改的原因主要是考虑到:如果子条件存在一条完整的SELECT语句,而这个完整的SELECT语句又不完全依赖于sub queryExpression 需要其它变量做拼接条件,这种情况这么改其实是不太好的。虽说有些过度臆想。我认为QueryCond不应当作为SqlSegment的实现,应当把这一块抽离出来,然后将一系列的条件,按照where、groupby、orderby、having分开以不同片段组的形式聚合到QueryCond中。查询条件的拼接和确定应当在QueryCond完成,而不是在dao执行查询动作的时候再去拼接,如果我想复用某个QueryCond的话,它的这个拼接动作会多次重复的发生,这其实是不合理的。

@afumu
Copy link
Member

afumu commented Sep 18, 2023

@DeerSunny 确实有问题,可以这么判断
image

最新的代码,修改还不对 2023/09/15 18:06:23 D:/test/gorm-plus/gplus/dao.go:197 [0.541ms] [rows:0] SELECT * FROM Users WHERE username = 'afumu' OR age = 20 OR --- FAIL: TestSelectListOr2 (0.00s)

这方案我上面已经提到过了其实,当时不想这么改的原因主要是考虑到:如果子条件存在一条完整的SELECT语句,而这个完整的SELECT语句又不完全依赖于sub queryExpression 需要其它变量做拼接条件,这种情况这么改其实是不太好的。虽说有些过度臆想。我认为QueryCond不应当作为SqlSegment的实现,应当把这一块抽离出来,然后将一系列的条件,按照where、groupby、orderby、having分开以不同片段组的形式聚合到QueryCond中。查询条件的拼接和确定应当在QueryCond完成,而不是在dao执行查询动作的时候再去拼接,如果我想复用某个QueryCond的话,它的这个拼接动作会多次重复的发生,这其实是不合理的。

gplus前期是可以复用QueryCond,但是当时的设计使得用户没有办法使用自定义的db,灵活性很差,所以后面就修改到dao来拼接条件了,这样用户可以任意传入db或者增加db额外的条件。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants