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

Fix joins in db.Find(AndCount) #28978

Merged
merged 2 commits into from
Jan 30, 2024
Merged

Fix joins in db.Find(AndCount) #28978

merged 2 commits into from
Jan 30, 2024

Conversation

KN4CK3R
Copy link
Member

@KN4CK3R KN4CK3R commented Jan 29, 2024

No description provided.

@KN4CK3R KN4CK3R requested a review from lunny January 29, 2024 20:41
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 29, 2024
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 29, 2024
@@ -133,27 +133,28 @@ type FindOptionsOrder interface {

// Find represents a common find function which accept an options interface
func Find[T any](ctx context.Context, opts FindOptions) ([]*T, error) {
sess := GetEngine(ctx)
sess := GetEngine(ctx).Where(opts.ToConds())
Copy link
Member Author

Choose a reason for hiding this comment

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

JoinFunc is a func(sess Engine) which can be called with xorm.Engine and xorm.Session. Inside the join func sess.Join is used. Now the problem is that Session.Join adds the JOIN to the statement while Engine.Join creates a new Session which gets discarded afterwards. So calling the JoinFunc with xorm.Engine does "nothing" while calling with xorm.Session does what it should.

GetEngine(ctx) returns a xorm.Engine and Where(...) creates the needed Session.

Comment on lines +145 to +149
if orderOpt, ok := opts.(FindOptionsOrder); ok {
if order := orderOpt.ToOrders(); order != "" {
sess.OrderBy(order)
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Add order support for Find.

Comment on lines +192 to +198
if joinOpt, ok := opts.(FindOptionsJoin); ok {
for _, joinFunc := range joinOpt.ToJoins() {
if err := joinFunc(sess); err != nil {
return nil, 0, err
}
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Add join support for FindAndCount.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jan 29, 2024
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jan 30, 2024
@lunny lunny enabled auto-merge (squash) January 30, 2024 02:13
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jan 30, 2024
@lunny lunny merged commit 27d4c11 into go-gitea:main Jan 30, 2024
25 checks passed
@GiteaBot GiteaBot added this to the 1.22.0 milestone Jan 30, 2024
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jan 30, 2024
@KN4CK3R KN4CK3R deleted the fix-join branch January 30, 2024 11:16
henrygoodman pushed a commit to henrygoodman/gitea that referenced this pull request Jan 31, 2024
silverwind added a commit to silverwind/gitea that referenced this pull request Feb 1, 2024
* origin/main:
  [skip ci] Updated translations via Crowdin
  Fix UI Spacing Errors in mirror settings (go-gitea#28990)
  Add htmx guidelines (go-gitea#28993)
  Some refactor for git http (go-gitea#28995)
  Fix an actions schedule bug (go-gitea#28942)
  Fix doc img path in profile readme (go-gitea#28994)
  Introduce htmx and use it to avoid full page load on `Subscribe` and `Follow` (go-gitea#28908)
  Fix joins in `db.Find(AndCount)` (go-gitea#28978)
  Update golang links to use https (go-gitea#28980)
  Fix google logo in security page (go-gitea#28982)
zjjhot added a commit to zjjhot/gitea that referenced this pull request Feb 1, 2024
* giteaofficial/main:
  Revert "Speed up loading the dashboard on mysql/mariadb (go-gitea#28546)" (go-gitea#29006)
  Update dorny/paths-filter action (go-gitea#29003)
  [skip ci] Updated translations via Crowdin
  Fix UI Spacing Errors in mirror settings (go-gitea#28990)
  Add htmx guidelines (go-gitea#28993)
  Some refactor for git http (go-gitea#28995)
  Fix an actions schedule bug (go-gitea#28942)
  Fix doc img path in profile readme (go-gitea#28994)
  Introduce htmx and use it to avoid full page load on `Subscribe` and `Follow` (go-gitea#28908)
  Fix joins in `db.Find(AndCount)` (go-gitea#28978)
  Update golang links to use https (go-gitea#28980)
  Fix google logo in security page (go-gitea#28982)
  Also match weakly validated ETags (go-gitea#28957)
silverwind pushed a commit to silverwind/gitea that referenced this pull request Feb 20, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants