From d1271b13461fad054602f534dac853991addc5be Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Mon, 19 Oct 2020 21:53:12 +0100 Subject: [PATCH 01/11] When replying to an outdated comment it should not appear on the files page This happened because the comment took the latest commitID as its base instead of the reviewID that it was replying to. There was also no way of creating an already outdated comment - and a reply to a review on an outdated line should be outdated. Signed-off-by: Andrew Thornton --- models/issue_comment.go | 12 +++++ models/migrations/migrations.go | 2 + models/migrations/v158.go | 78 +++++++++++++++++++++++++++++++++ services/pull/review.go | 78 ++++++++++++++++++++++----------- 4 files changed, 145 insertions(+), 25 deletions(-) create mode 100644 models/migrations/v158.go diff --git a/models/issue_comment.go b/models/issue_comment.go index a7e9c049bf309..7bcea40b93006 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -712,6 +712,7 @@ func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err RefAction: opts.RefAction, RefIsPull: opts.RefIsPull, IsForcePush: opts.IsForcePush, + Invalidated: opts.Invalidated, } if _, err = e.Insert(comment); err != nil { return nil, err @@ -878,6 +879,7 @@ type CreateCommentOptions struct { RefAction references.XRefAction RefIsPull bool IsForcePush bool + Invalidated bool } // CreateComment creates comment of issue or commit. @@ -953,6 +955,8 @@ type FindCommentsOptions struct { ReviewID int64 Since int64 Before int64 + Line int64 + TreePath string Type CommentType } @@ -976,6 +980,12 @@ func (opts *FindCommentsOptions) toConds() builder.Cond { if opts.Type != CommentTypeUnknown { cond = cond.And(builder.Eq{"comment.type": opts.Type}) } + if opts.Line > 0 { + cond = cond.And(builder.Eq{"comment.line": opts.Line}) + } + if len(opts.TreePath) > 0 { + cond = cond.And(builder.Eq{"comment.tree_path": opts.TreePath}) + } return cond } @@ -990,6 +1000,8 @@ func findComments(e Engine, opts FindCommentsOptions) ([]*Comment, error) { sess = opts.setSessionPagination(sess) } + // WARNING: If you change this order you will need to fix createCodeComment + return comments, sess. Asc("comment.created_unix"). Asc("comment.id"). diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index cbf8ae87327ea..4715f192c157e 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -250,6 +250,8 @@ var migrations = []Migration{ NewMigration("fix publisher ID for tag releases", fixPublisherIDforTagReleases), // v157 -> v158 NewMigration("ensure repo topics are up-to-date", fixRepoTopics), + // v158 -> v159 + NewMigration("code comment replies should have the commitID of the review they are replying to", updateCodeCommentReplies), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v158.go b/models/migrations/v158.go new file mode 100644 index 0000000000000..ce67fb8be4c6b --- /dev/null +++ b/models/migrations/v158.go @@ -0,0 +1,78 @@ +// Copyright 2020 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package migrations + +import ( + "code.gitea.io/gitea/modules/log" + + "xorm.io/xorm" +) + +func updateCodeCommentReplies(x *xorm.Engine) error { + type Comment struct { + ID int64 `xorm:"pk autoincr"` + CommitSHA string `xorm:"VARCHAR(40)"` + Patch string `xorm:"TEXT patch"` + Invalidated bool + + // Not extracted but used in the below query + Type int `xorm:"INDEX"` + Line int64 // - previous line / + proposed line + TreePath string + ReviewID int64 `xorm:"index"` + } + + if err := x.Sync2(new(Comment)); err != nil { + return err + } + + sess := x.NewSession() + defer sess.Close() + if err := sess.Begin(); err != nil { + return err + } + + var start = 0 + var batchSize = 100 + for { + var comments = make([]*Comment, 0, batchSize) + if err := sess.SQL(`SELECT comment.id as id, first.commit_sha as commit_sha, first.patch as patch, first.invalidated as invalidated + FROM comment INNER JOIN ( + SELECT C.id, C.review_id, C.line, C.tree_path, C.patch, C.commit_sha, C.invalidated + FROM comment AS C + WHERE C.type = 21 + AND C.created_unix = + (SELECT MIN(comment.created_unix) + FROM comment + WHERE comment.review_id = C.review_id + AND comment.type = 21 + AND comment.line = C.line + AND comment.tree_path = C.tree_path) + ) AS first + ON comment.review_id = first.review_id + AND comment.tree_path = first.tree_path AND comment.line = first.line + WHERE comment.type = 21 + AND comment.id != first.id + AND comment.commit_sha != first.commit_sha`).Limit(batchSize, start).Find(&comments); err != nil { + log.Error("failed to select: %v", err) + return err + } + + for _, comment := range comments { + if _, err := sess.Table("comment").Cols("commit_sha", "patch", "invalidated").Update(comment); err != nil { + log.Error("failed to update comment[%d]: %v %v", comment.ID, comment, err) + return err + } + } + + start += len(comments) + + if len(comments) < batchSize { + break + } + } + + return sess.Commit() +} diff --git a/services/pull/review.go b/services/pull/review.go index 99afdd73c2150..356e319003130 100644 --- a/services/pull/review.go +++ b/services/pull/review.go @@ -122,41 +122,69 @@ func createCodeComment(doer *models.User, repo *models.Repository, issue *models } defer gitRepo.Close() - // FIXME validate treePath - // Get latest commit referencing the commented line - // No need for get commit for base branch changes + invalidated := false + head := pr.GetGitRefName() if line > 0 { - commit, err := gitRepo.LineBlame(pr.GetGitRefName(), gitRepo.Path, treePath, uint(line)) - if err == nil { - commitID = commit.ID.String() - } else if !(strings.Contains(err.Error(), "exit status 128 - fatal: no such path") || notEnoughLines.MatchString(err.Error())) { - return nil, fmt.Errorf("LineBlame[%s, %s, %s, %d]: %v", pr.GetGitRefName(), gitRepo.Path, treePath, line, err) + if reviewID != 0 { + first, err := models.FindComments(models.FindCommentsOptions{ + ReviewID: reviewID, + Line: line, + TreePath: treePath, + Type: models.CommentTypeCode, + ListOptions: models.ListOptions{ + PageSize: 1, + Page: 1, + }, + }) + if err == nil && len(first) > 0 { + commitID = first[0].CommitSHA + invalidated = first[0].Invalidated + patch = first[0].Patch + } else if err != nil && !models.IsErrCommentNotExist(err) { + return nil, fmt.Errorf("Find first comment for %d line %d path %s. Error: %v", reviewID, line, treePath, err) + } else { + review, err := models.GetReviewByID(reviewID) + if err == nil && len(review.CommitID) > 0 { + head = review.CommitID + } else if err != nil && !models.IsErrReviewNotExist(err) { + return nil, fmt.Errorf("GetReviewByID %d. Error: %v", reviewID, err) + } + } + } + + if len(commitID) == 0 { + // FIXME validate treePath + // Get latest commit referencing the commented line + // No need for get commit for base branch changes + commit, err := gitRepo.LineBlame(head, gitRepo.Path, treePath, uint(line)) + if err == nil { + commitID = commit.ID.String() + } else if !(strings.Contains(err.Error(), "exit status 128 - fatal: no such path") || notEnoughLines.MatchString(err.Error())) { + return nil, fmt.Errorf("LineBlame[%s, %s, %s, %d]: %v", pr.GetGitRefName(), gitRepo.Path, treePath, line, err) + } } } // Only fetch diff if comment is review comment - if reviewID != 0 { - headCommitID, err := gitRepo.GetRefCommitID(pr.GetGitRefName()) - if err != nil { - return nil, fmt.Errorf("GetRefCommitID[%s]: %v", pr.GetGitRefName(), err) - } + if len(patch) == 0 && reviewID != 0 { patchBuf := new(bytes.Buffer) - if err := git.GetRepoRawDiffForFile(gitRepo, pr.MergeBase, headCommitID, git.RawDiffNormal, treePath, patchBuf); err != nil { - return nil, fmt.Errorf("GetRawDiffForLine[%s, %s, %s, %s]: %v", err, gitRepo.Path, pr.MergeBase, headCommitID, treePath) + if err := git.GetRepoRawDiffForFile(gitRepo, pr.MergeBase, commitID, git.RawDiffNormal, treePath, patchBuf); err != nil { + return nil, fmt.Errorf("GetRawDiffForLine[%s, %s, %s, %s]: %v", err, gitRepo.Path, pr.MergeBase, commitID, treePath) } patch = git.CutDiffAroundLine(patchBuf, int64((&models.Comment{Line: line}).UnsignedLine()), line < 0, setting.UI.CodeCommentLines) } return models.CreateComment(&models.CreateCommentOptions{ - Type: models.CommentTypeCode, - Doer: doer, - Repo: repo, - Issue: issue, - Content: content, - LineNum: line, - TreePath: treePath, - CommitSHA: commitID, - ReviewID: reviewID, - Patch: patch, + Type: models.CommentTypeCode, + Doer: doer, + Repo: repo, + Issue: issue, + Content: content, + LineNum: line, + TreePath: treePath, + CommitSHA: commitID, + ReviewID: reviewID, + Patch: patch, + Invalidated: invalidated, }) } From 4155315e43b6209448a3c62a03f6821ec7d8b97f Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Tue, 20 Oct 2020 17:54:45 +0100 Subject: [PATCH 02/11] fix test Signed-off-by: Andrew Thornton --- services/pull/review.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/services/pull/review.go b/services/pull/review.go index 356e319003130..f0ee234a42419 100644 --- a/services/pull/review.go +++ b/services/pull/review.go @@ -167,9 +167,16 @@ func createCodeComment(doer *models.User, repo *models.Repository, issue *models // Only fetch diff if comment is review comment if len(patch) == 0 && reviewID != 0 { + if len(commitID) == 0 { + commitID, err = gitRepo.GetRefCommitID(pr.GetGitRefName()) + if err != nil { + return nil, fmt.Errorf("GetRefCommitID[%s]: %v", pr.GetGitRefName(), err) + } + } + patchBuf := new(bytes.Buffer) if err := git.GetRepoRawDiffForFile(gitRepo, pr.MergeBase, commitID, git.RawDiffNormal, treePath, patchBuf); err != nil { - return nil, fmt.Errorf("GetRawDiffForLine[%s, %s, %s, %s]: %v", err, gitRepo.Path, pr.MergeBase, commitID, treePath) + return nil, fmt.Errorf("GetRawDiffForLine[%s, %s, %s, %s]: %v", gitRepo.Path, pr.MergeBase, commitID, treePath, err) } patch = git.CutDiffAroundLine(patchBuf, int64((&models.Comment{Line: line}).UnsignedLine()), line < 0, setting.UI.CodeCommentLines) } From d54dfb45bd8919d79141350ce6f380d9c1d5173c Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Fri, 6 Nov 2020 18:16:59 +0000 Subject: [PATCH 03/11] Fix broken migration Signed-off-by: Andrew Thornton --- models/migrations/v158.go | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/models/migrations/v158.go b/models/migrations/v158.go index ce67fb8be4c6b..bdaec1314d6d9 100644 --- a/models/migrations/v158.go +++ b/models/migrations/v158.go @@ -5,7 +5,11 @@ package migrations import ( + "fmt" + "strconv" + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/setting" "xorm.io/xorm" ) @@ -38,7 +42,8 @@ func updateCodeCommentReplies(x *xorm.Engine) error { var batchSize = 100 for { var comments = make([]*Comment, 0, batchSize) - if err := sess.SQL(`SELECT comment.id as id, first.commit_sha as commit_sha, first.patch as patch, first.invalidated as invalidated + + sqlCmd := `SELECT comment.id as id, first.commit_sha as commit_sha, first.patch as patch, first.invalidated as invalidated FROM comment INNER JOIN ( SELECT C.id, C.review_id, C.line, C.tree_path, C.patch, C.commit_sha, C.invalidated FROM comment AS C @@ -55,13 +60,29 @@ func updateCodeCommentReplies(x *xorm.Engine) error { AND comment.tree_path = first.tree_path AND comment.line = first.line WHERE comment.type = 21 AND comment.id != first.id - AND comment.commit_sha != first.commit_sha`).Limit(batchSize, start).Find(&comments); err != nil { + AND comment.commit_sha != first.commit_sha` + + switch { + case setting.Database.UseMySQL: + sqlCmd += " LIMIT " + strconv.Itoa(batchSize) + ", " + strconv.Itoa(start) + case setting.Database.UsePostgreSQL: + fallthrough + case setting.Database.UseSQLite3: + sqlCmd += " LIMIT " + strconv.Itoa(batchSize) + " OFFSET " + strconv.Itoa(start) + case setting.Database.UseMSSQL: + sqlCmd = "SELECT TOP " + strconv.Itoa(batchSize) + sqlCmd[6:] + + "(id NOT IN ( SELECT TOP " + strconv.Itoa(start) + sqlCmd[6:] + "))" + default: + return fmt.Errorf("Unsupported database type") + } + + if err := sess.SQL(sqlCmd).Find(&comments); err != nil { log.Error("failed to select: %v", err) return err } for _, comment := range comments { - if _, err := sess.Table("comment").Cols("commit_sha", "patch", "invalidated").Update(comment); err != nil { + if _, err := sess.Table("comment").ID(comment.ID).Cols("commit_sha", "patch", "invalidated").Update(comment); err != nil { log.Error("failed to update comment[%d]: %v %v", comment.ID, comment, err) return err } From 43897c051e952ee6a1840f782714ed4da75378fd Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Fri, 6 Nov 2020 20:08:18 +0000 Subject: [PATCH 04/11] fix mssql Signed-off-by: Andrew Thornton --- models/migrations/v158.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/migrations/v158.go b/models/migrations/v158.go index bdaec1314d6d9..0a2608b765197 100644 --- a/models/migrations/v158.go +++ b/models/migrations/v158.go @@ -71,7 +71,7 @@ func updateCodeCommentReplies(x *xorm.Engine) error { sqlCmd += " LIMIT " + strconv.Itoa(batchSize) + " OFFSET " + strconv.Itoa(start) case setting.Database.UseMSSQL: sqlCmd = "SELECT TOP " + strconv.Itoa(batchSize) + sqlCmd[6:] + - "(id NOT IN ( SELECT TOP " + strconv.Itoa(start) + sqlCmd[6:] + "))" + " AND (id NOT IN ( SELECT TOP " + strconv.Itoa(start) + sqlCmd[6:] + "))" default: return fmt.Errorf("Unsupported database type") } From 17aa688be2f6e7d830135f5a95bbdfeab09dde91 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sat, 7 Nov 2020 18:45:21 +0000 Subject: [PATCH 05/11] Create temporary table because ... well MSSQL ... Signed-off-by: Andrew Thornton --- models/migrations/v158.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/models/migrations/v158.go b/models/migrations/v158.go index 0a2608b765197..994911ffd6b2b 100644 --- a/models/migrations/v158.go +++ b/models/migrations/v158.go @@ -43,8 +43,8 @@ func updateCodeCommentReplies(x *xorm.Engine) error { for { var comments = make([]*Comment, 0, batchSize) - sqlCmd := `SELECT comment.id as id, first.commit_sha as commit_sha, first.patch as patch, first.invalidated as invalidated - FROM comment INNER JOIN ( + sqlSelect := `SELECT comment.id as id, first.commit_sha as commit_sha, first.patch as patch, first.invalidated as invalidated` + sqlTail := ` FROM comment INNER JOIN ( SELECT C.id, C.review_id, C.line, C.tree_path, C.patch, C.commit_sha, C.invalidated FROM comment AS C WHERE C.type = 21 @@ -62,6 +62,7 @@ func updateCodeCommentReplies(x *xorm.Engine) error { AND comment.id != first.id AND comment.commit_sha != first.commit_sha` + sqlCmd := sqlSelect + sqlTail switch { case setting.Database.UseMySQL: sqlCmd += " LIMIT " + strconv.Itoa(batchSize) + ", " + strconv.Itoa(start) @@ -70,8 +71,12 @@ func updateCodeCommentReplies(x *xorm.Engine) error { case setting.Database.UseSQLite3: sqlCmd += " LIMIT " + strconv.Itoa(batchSize) + " OFFSET " + strconv.Itoa(start) case setting.Database.UseMSSQL: - sqlCmd = "SELECT TOP " + strconv.Itoa(batchSize) + sqlCmd[6:] + - " AND (id NOT IN ( SELECT TOP " + strconv.Itoa(start) + sqlCmd[6:] + "))" + if _, err := sess.Exec(sqlSelect + " INTO temp_comments" + sqlTail); err != nil { + log.Error("unable to create temporary table") + return err + } + sqlCmd = "SELECT TOP " + strconv.Itoa(batchSize) + " FROM temp_comments WHERE " + + "(id NOT IN ( SELECT TOP " + strconv.Itoa(start) + " FROM temp_comments))" default: return fmt.Errorf("Unsupported database type") } From 078c409bac60a13fb926895d6042217f4fa66d5a Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sat, 7 Nov 2020 18:45:55 +0000 Subject: [PATCH 06/11] Create temporary table because ... well MSSQL ... Signed-off-by: Andrew Thornton --- models/migrations/v158.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/migrations/v158.go b/models/migrations/v158.go index 994911ffd6b2b..0b62f65ce08a5 100644 --- a/models/migrations/v158.go +++ b/models/migrations/v158.go @@ -71,7 +71,7 @@ func updateCodeCommentReplies(x *xorm.Engine) error { case setting.Database.UseSQLite3: sqlCmd += " LIMIT " + strconv.Itoa(batchSize) + " OFFSET " + strconv.Itoa(start) case setting.Database.UseMSSQL: - if _, err := sess.Exec(sqlSelect + " INTO temp_comments" + sqlTail); err != nil { + if _, err := sess.Exec(sqlSelect + " INTO #temp_comments" + sqlTail); err != nil { log.Error("unable to create temporary table") return err } From 2e07d053be4edc3e4e59b7d9ee8439759af30125 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sat, 7 Nov 2020 19:15:31 +0000 Subject: [PATCH 07/11] Create temporary table because ... well MSSQL ... Signed-off-by: Andrew Thornton --- models/migrations/v158.go | 56 +++++++++++++++++++++------------------ 1 file changed, 30 insertions(+), 26 deletions(-) diff --git a/models/migrations/v158.go b/models/migrations/v158.go index 0b62f65ce08a5..1ee3ebc0edeaf 100644 --- a/models/migrations/v158.go +++ b/models/migrations/v158.go @@ -32,37 +32,45 @@ func updateCodeCommentReplies(x *xorm.Engine) error { return err } + sqlSelect := `SELECT comment.id as id, first.commit_sha as commit_sha, first.patch as patch, first.invalidated as invalidated` + sqlTail := ` FROM comment INNER JOIN ( + SELECT C.id, C.review_id, C.line, C.tree_path, C.patch, C.commit_sha, C.invalidated + FROM comment AS C + WHERE C.type = 21 + AND C.created_unix = + (SELECT MIN(comment.created_unix) + FROM comment + WHERE comment.review_id = C.review_id + AND comment.type = 21 + AND comment.line = C.line + AND comment.tree_path = C.tree_path) + ) AS first + ON comment.review_id = first.review_id + AND comment.tree_path = first.tree_path AND comment.line = first.line + WHERE comment.type = 21 + AND comment.id != first.id + AND comment.commit_sha != first.commit_sha` + + sqlCmd := sqlSelect + sqlTail + sess := x.NewSession() defer sess.Close() if err := sess.Begin(); err != nil { return err } + if setting.Database.UseMSSQL { + if _, err := sess.Exec(sqlSelect + " INTO #temp_comments" + sqlTail); err != nil { + log.Error("unable to create temporary table") + return err + } + } + var start = 0 var batchSize = 100 for { var comments = make([]*Comment, 0, batchSize) - sqlSelect := `SELECT comment.id as id, first.commit_sha as commit_sha, first.patch as patch, first.invalidated as invalidated` - sqlTail := ` FROM comment INNER JOIN ( - SELECT C.id, C.review_id, C.line, C.tree_path, C.patch, C.commit_sha, C.invalidated - FROM comment AS C - WHERE C.type = 21 - AND C.created_unix = - (SELECT MIN(comment.created_unix) - FROM comment - WHERE comment.review_id = C.review_id - AND comment.type = 21 - AND comment.line = C.line - AND comment.tree_path = C.tree_path) - ) AS first - ON comment.review_id = first.review_id - AND comment.tree_path = first.tree_path AND comment.line = first.line - WHERE comment.type = 21 - AND comment.id != first.id - AND comment.commit_sha != first.commit_sha` - - sqlCmd := sqlSelect + sqlTail switch { case setting.Database.UseMySQL: sqlCmd += " LIMIT " + strconv.Itoa(batchSize) + ", " + strconv.Itoa(start) @@ -71,12 +79,8 @@ func updateCodeCommentReplies(x *xorm.Engine) error { case setting.Database.UseSQLite3: sqlCmd += " LIMIT " + strconv.Itoa(batchSize) + " OFFSET " + strconv.Itoa(start) case setting.Database.UseMSSQL: - if _, err := sess.Exec(sqlSelect + " INTO #temp_comments" + sqlTail); err != nil { - log.Error("unable to create temporary table") - return err - } - sqlCmd = "SELECT TOP " + strconv.Itoa(batchSize) + " FROM temp_comments WHERE " + - "(id NOT IN ( SELECT TOP " + strconv.Itoa(start) + " FROM temp_comments))" + sqlCmd = "SELECT TOP " + strconv.Itoa(batchSize) + " FROM #temp_comments WHERE " + + "(id NOT IN ( SELECT TOP " + strconv.Itoa(start) + " FROM #temp_comments))" default: return fmt.Errorf("Unsupported database type") } From aab6e2678450696acb3676b5c65dd587af7cda7a Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sat, 7 Nov 2020 21:33:32 +0000 Subject: [PATCH 08/11] fix mssql Signed-off-by: Andrew Thornton --- models/migrations/v158.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/migrations/v158.go b/models/migrations/v158.go index 1ee3ebc0edeaf..2006a973285a1 100644 --- a/models/migrations/v158.go +++ b/models/migrations/v158.go @@ -79,8 +79,8 @@ func updateCodeCommentReplies(x *xorm.Engine) error { case setting.Database.UseSQLite3: sqlCmd += " LIMIT " + strconv.Itoa(batchSize) + " OFFSET " + strconv.Itoa(start) case setting.Database.UseMSSQL: - sqlCmd = "SELECT TOP " + strconv.Itoa(batchSize) + " FROM #temp_comments WHERE " + - "(id NOT IN ( SELECT TOP " + strconv.Itoa(start) + " FROM #temp_comments))" + sqlCmd = "SELECT TOP " + strconv.Itoa(batchSize) + " * FROM #temp_comments WHERE " + + "(id NOT IN ( SELECT TOP " + strconv.Itoa(start) + " id FROM #temp_comments ORDER BY id )) ORDER BY id" default: return fmt.Errorf("Unsupported database type") } From c015454ea6f39393461173b0e5e6f5c2da387462 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 8 Nov 2020 15:32:33 +0000 Subject: [PATCH 09/11] move session within the batch Signed-off-by: Andrew Thornton --- models/migrations/v158.go | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/models/migrations/v158.go b/models/migrations/v158.go index 2006a973285a1..07497cda7efd4 100644 --- a/models/migrations/v158.go +++ b/models/migrations/v158.go @@ -53,22 +53,22 @@ func updateCodeCommentReplies(x *xorm.Engine) error { sqlCmd := sqlSelect + sqlTail - sess := x.NewSession() - defer sess.Close() - if err := sess.Begin(); err != nil { - return err - } - - if setting.Database.UseMSSQL { - if _, err := sess.Exec(sqlSelect + " INTO #temp_comments" + sqlTail); err != nil { - log.Error("unable to create temporary table") - return err - } - } - var start = 0 var batchSize = 100 for { + sess := x.NewSession() + defer sess.Close() + if err := sess.Begin(); err != nil { + return err + } + + if setting.Database.UseMSSQL { + if _, err := sess.Exec(sqlSelect + " INTO #temp_comments" + sqlTail); err != nil { + log.Error("unable to create temporary table") + return err + } + } + var comments = make([]*Comment, 0, batchSize) switch { @@ -99,10 +99,13 @@ func updateCodeCommentReplies(x *xorm.Engine) error { start += len(comments) + if err := sess.Commit(); err != nil { + return err + } if len(comments) < batchSize { break } } - return sess.Commit() + return nil } From 560ee611b147ad78c91660bc574e9fce32ed1c8b Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 8 Nov 2020 15:34:42 +0000 Subject: [PATCH 10/11] regen the sqlcmd each time round the loop Signed-off-by: Andrew Thornton --- models/migrations/v158.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/models/migrations/v158.go b/models/migrations/v158.go index 07497cda7efd4..610385f8212c5 100644 --- a/models/migrations/v158.go +++ b/models/migrations/v158.go @@ -51,8 +51,7 @@ func updateCodeCommentReplies(x *xorm.Engine) error { AND comment.id != first.id AND comment.commit_sha != first.commit_sha` - sqlCmd := sqlSelect + sqlTail - + var sqlCmd string var start = 0 var batchSize = 100 for { @@ -73,11 +72,11 @@ func updateCodeCommentReplies(x *xorm.Engine) error { switch { case setting.Database.UseMySQL: - sqlCmd += " LIMIT " + strconv.Itoa(batchSize) + ", " + strconv.Itoa(start) + sqlCmd = sqlSelect + sqlTail + " LIMIT " + strconv.Itoa(batchSize) + ", " + strconv.Itoa(start) case setting.Database.UsePostgreSQL: fallthrough case setting.Database.UseSQLite3: - sqlCmd += " LIMIT " + strconv.Itoa(batchSize) + " OFFSET " + strconv.Itoa(start) + sqlCmd = sqlSelect + sqlTail + " LIMIT " + strconv.Itoa(batchSize) + " OFFSET " + strconv.Itoa(start) case setting.Database.UseMSSQL: sqlCmd = "SELECT TOP " + strconv.Itoa(batchSize) + " * FROM #temp_comments WHERE " + "(id NOT IN ( SELECT TOP " + strconv.Itoa(start) + " id FROM #temp_comments ORDER BY id )) ORDER BY id" From e6152e64d9d9b571e4bc415d7f5f1a9f89bb59b2 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 8 Nov 2020 16:28:48 +0000 Subject: [PATCH 11/11] as per @lunny Signed-off-by: Andrew Thornton --- models/migrations/v158.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/migrations/v158.go b/models/migrations/v158.go index 610385f8212c5..a6f2178f87442 100644 --- a/models/migrations/v158.go +++ b/models/migrations/v158.go @@ -54,9 +54,9 @@ func updateCodeCommentReplies(x *xorm.Engine) error { var sqlCmd string var start = 0 var batchSize = 100 + sess := x.NewSession() + defer sess.Close() for { - sess := x.NewSession() - defer sess.Close() if err := sess.Begin(); err != nil { return err }