Where clauses show up as nil in Scopes

[acidjazz]

GORM Playground Link

https://github.com/go-gorm/playground/pull/592

Description

say i have this scope

func SomeScope(t *testing.T) func(d *gorm.DB) *gorm.DB {
    return func(d *gorm.DB) *gorm.DB {
        session := d.Session(&gorm.Session{})
        if session.Statement.Clauses["WHERE"].Expression == nil {
            t.Errorf("Should not be nil")
        }
        return d
    }
}

and i assign it and then specify a Where clause:

    var userNil User
    query := DB.Scopes(SomeScope(t)).Where("Name NOT NULL")
    if query.Statement.Clauses["WHERE"].Expression == nil {
        t.Errorf("Should not be nil")
    }

    query.Find(&userNil)

the clause is nil in session.Statement.Clauses but not in query.Statement.Clauses

[black-06]

Yes, we cleared the condition for the Scopes db param, see #6152 . Because we don't want multiple scopes to influence each other.

[acidjazz]

@jinzhu @black-06 I see, are you sure this is the proper way to go about this fix? I don't think removing/clearing this is really a solution, since it now removes a major feature of the Scope. Can we maybe at least have a way to turn this on?

I think it would be very beneficial to be able to detect and react to clauses, especially WHERE clauses, when you have functionality like pagination, where these clauses can effect things like total rows returned.

Maybe is there any other way I can detect and react to these clauses in my scope? I think not being able to work with these really limits what you can do with a scope

[black-06]

@jinzhu @black-06 I see, are you sure this is the proper way to go about this fix?

Umm... I don't have a better idea, do you have any suggestions about this fix 😀?

Can we maybe at least have a way to turn this on?

Add a switch for it? @a631807682

Is there any way I can detect and react to these clauses in my scope ?? I think not being able to work with these in a Scope really limits what you can do with a scope

we can't get WHERE clauses in Scope now...

[acidjazz]

Thank you for the immediate response

we can't get WHERE clauses in Scope now...

Does this mean if I change to a version of gorm before this patch I'll be able to see these clauses?

Add a switch for it? @a631807682

Maybe something like KeepExpressions or AllowMultipleExpressions, no opinion on default, just need to be able to have it

Is this the change that did it? https://github.com/go-gorm/gorm/pull/6152/files#diff-fe4d8b9673d524cf5dd275564a86eabe3bad07315c9f27d42e38ec071e57354dR341

[black-06]

Does this mean if I change to a version of gorm before this patch I'll be able to see these clauses?

Of course.

Is this the change that did it?

https://github.com/go-gorm/gorm/pull/6152/files#diff-5716ee44859beee3016f8f6c7ed2ffc79d0ac8e79f59e0a05ef76add9f1736a9R380

https://github.com/go-gorm/gorm/blob/e61b98d69677b8871d832baf2489942d79054a4a/chainable_api.go#L378-L382

[a631807682]

I don't think it's a good idea to add a switch. First of all, the change here is because Clauses["WHERE"] is incorrect, and relying on an incorrect Clauses["WHERE"] for subsequent reactions is likely to get an incorrect result. . Can you provide a real example where using Clauses["WHERE"] is necessary?

[acidjazz]

I don't think it's a good idea to add a switch. First of all, the change here is because Clauses["WHERE"] is incorrect, and relying on an incorrect Clauses["WHERE"] for subsequent reactions is likely to get an incorrect result. . Can you provide a real example where using Clauses["WHERE"] is necessary?

Yes, I put it in my 1st comment and the reason I created this issue, Pagination, if you do any Where clause you're changing the row count your pagination needs.

I think it is a correct Clauses["WHERE"] as long as you know not to stack Where scopes, and making it nil is worse. I think the switch is necessary at the least.

[acidjazz]

I just don't understand where the solution of multiple Where clauses interfering is to eliminate the functionality of detecting them in a Scope, I thought this was a major reason Scopes exist and how they work. I don't see how simple things like pagination can exist w/out having this.

[acidjazz]

@black-06 any way we can get this going as a switch? should i try and go for a PR? i'd really like proper pagination and to not have to re-build my own queries

I'd be happy to get a PR going , and ofcourse we can default it to off

[black-06]

I don't quite understand how you use Scope for paging. could you please provide code?

[acidjazz]

// Paginate - for why where is being passed in: https://github.com/go-gorm/gorm/issues/6281
func Paginate(pagination *Pagination, wheres ...string) func(db *gorm.DB) *gorm.DB {
    return func(db *gorm.DB) *gorm.DB {
        var totalRows int64
        instance := db.Session(&gorm.Session{Initialized: true}).Model(db.Statement.Model)
        for _, where := range wheres {
            instance.Where(where)
        }
        instance.Count(&totalRows)
        pagination.TotalRows = int(totalRows)
        pagination.TotalPages = int(math.Ceil(float64(totalRows) / float64(pagination.Limit)))
        pagination.Pages = GetPages(pagination.ShowPages, pagination.GetPage(), pagination.TotalPages, pagination.GetMaxPages())
        pagination.FirstItem, pagination.LastItem = SetItems(pagination.GetPage(), pagination.Limit, pagination.TotalRows)
        return db.Offset(pagination.GetOffset()).Limit(pagination.GetLimit()).Order(pagination.GetSort())
    }
}

this function had hopes that the wheres were pulled in from gorm.DB, but since they were removed you have to manually pass them in:

    var activities []models.Activity
    paginated := paginate.Pagination{}
    whereAdmin := "admin = false"
    whereUsers := fmt.Sprintf("user_id IN (%s)", paginate.UIntJoin(userIds, ","))
    paginated.SetPage(params.Page).SetSort("created_at").SetOrder("desc").SetLimit(params.Limit)
    database.Db.
        Preload("User").
        Scopes(paginate.Paginate(&paginated, whereAdmin, whereUsers)).
        Where("admin = ?", false).
        Where("user_id IN ?", userIds).
        Find(&activities)

[black-06]

This pager looks exquisite, but as a631807682 says

Clauses["WHERE"] may be incorrect.

For example, db.Scopes(paginate.Paginate(&paginated)).Scopes(SomeFuncAddWhere) once the Paginate is not the last scope func to be called, it will miss the conditions added in the subsequent scope func.

[acidjazz]

This pager looks exquisite, but as a631807682 says

Clauses["WHERE"] may be incorrect.

For example, db.Scopes(paginate.Paginate(&paginated)).Scopes(SomeFuncAddWhere) once the Paginate is not the last scope func to be called, it will miss the conditions added in the subsequent scope func.

That's fine, i can properly document how to use it, at least it'll work though.