acmestack / gorm-plus

Gorm-plus是基于Gorm的增强版,类似Mybatis-plus语法。Gorm-plus is based on an enhanced version of Gorm, similar to Mybatis-plus syntax.
https://github.com/acmestack/gorm-plus/wiki
Apache License 2.0
285 stars 44 forks source link

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

Open PhoenixL0911 opened 1 year ago

PhoenixL0911 commented 1 year ago

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 commented 1 year ago

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

func TestSelectListOr2(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))

}

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 commented 1 year ago

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

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()这个函数去解决。

aixj1984 commented 1 year ago

最新的代码,修改还不对 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 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

@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额外的条件。