Refactor part 2 - separate merge from diff iterator [change-log] optimize merge by 20 percent

cmd/lakectl/cmd/merge.go

@@ -10,13 +10,7 @@ const (
 	mergeCmdMaxArgs = 2
 )
 
-var mergeCreateTemplate = `Merged "{{.Merge.FromRef|yellow}}" into "{{.Merge.ToRef|yellow}}" to get "{{.Result.Reference|green}}".
-
-Added: {{.Result.Summary.Added}}
-Changed: {{.Result.Summary.Changed}}
-Removed: {{.Result.Summary.Removed}}
-
-`
+var mergeCreateTemplate = `Merged "{{.Merge.FromRef|yellow}}" into "{{.Merge.ToRef|yellow}}" to get "{{.Result.Reference|green}}".`
 
 type FromTo struct {
 	FromRef, ToRef string

nessie/identity_test.go

@@ -53,7 +53,7 @@ func TestIdentity(t *testing.T) {
 
 			resp, err := client.MergeIntoBranchWithResponse(ctx, repo, branch1, branch2, api.MergeIntoBranchJSONRequestBody{})
 			require.NoError(t, err, "error during merge")
-			require.NotNil(t, resp.JSON400, "merge should fail since there are no changes between the branches")
+			require.NotEmpty(t, resp.JSON200, "allow merge with no changes between the branches")
 		})
 	}
 }

pkg/api/controller.go

@@ -2791,37 +2791,13 @@ func (c *Controller) MergeIntoBranch(w http.ResponseWriter, r *http.Request, bod
 		writeError(w, http.StatusPreconditionFailed, err)
 		return
 	case errors.Is(err, catalog.ErrConflictFound) || errors.Is(err, graveler.ErrConflictFound):
-		writeResponse(w, http.StatusConflict, newMergeResultFromCatalog(res))
+		writeResponse(w, http.StatusConflict, MergeResult{Reference: res.Reference})
 		return
 	}
 	if handleAPIError(w, err) {
 		return
 	}
-
-	response := newMergeResultFromCatalog(res)
-	writeResponse(w, http.StatusOK, response)
-}
-
-func newMergeResultFromCatalog(res *catalog.MergeResult) MergeResult {
-	if res == nil {
-		return MergeResult{}
-	}
-	result := MergeResult{
-		Reference: res.Reference,
-	}
-	for k, v := range res.Summary {
-		switch k {
-		case catalog.DifferenceTypeAdded:
-			result.Summary.Added = v
-		case catalog.DifferenceTypeChanged:
-			result.Summary.Changed = v
-		case catalog.DifferenceTypeRemoved:
-			result.Summary.Removed = v
-		case catalog.DifferenceTypeConflict:
-			result.Summary.Conflict = v
-		}
-	}
-	return result
+	writeResponse(w, http.StatusOK, MergeResult{Reference: res.Reference}) // optimize returning unknown summary = 0
 }
 
 func (c *Controller) ListTags(w http.ResponseWriter, r *http.Request, repository string, params ListTagsParams) {

pkg/api/controller_test.go

@@ -342,7 +342,8 @@ func TestController_CommitsGetBranchCommitLogByPath(t *testing.T) {
 		user:         "user3",
 		commitName:   "P",
 	})
-	mergeCommit, _ := deps.catalog.Merge(ctx, "repo3", "main", "branch-b", "user3", "commitR", nil)
+	mergeCommit, err := deps.catalog.Merge(ctx, "repo3", "main", "branch-b", "user3", "commitR", nil)
+	testutil.Must(t, err)
 	commitsMap["commitR"] = mergeCommit.Reference
 	commitsMap["commitM"] = testCommitEntries(t, ctx, deps.catalog, deps, commitEntriesParams{
 		repo:         "repo3",
@@ -352,7 +353,8 @@ func TestController_CommitsGetBranchCommitLogByPath(t *testing.T) {
 		user:         "user2",
 		commitName:   "M",
 	})
-	mergeCommit, _ = deps.catalog.Merge(ctx, "repo3", "main", "branch-a", "user2", "commitN", nil)
+	mergeCommit, err = deps.catalog.Merge(ctx, "repo3", "main", "branch-a", "user2", "commitN", nil)
+	testutil.Must(t, err)
 	commitsMap["commitN"] = mergeCommit.Reference
 	commitsMap["commitX"] = testCommitEntries(t, ctx, deps.catalog, deps, commitEntriesParams{
 		repo:         "repo3",

pkg/catalog/catalog.go

@@ -985,7 +985,7 @@ func (c *Catalog) Revert(ctx context.Context, repository string, branch string,
 	}); err != nil {
 		return err
 	}
-	_, _, err := c.Store.Revert(ctx, repositoryID, branchID, reference, parentNumber, commitParams)
+	_, err := c.Store.Revert(ctx, repositoryID, branchID, reference, parentNumber, commitParams)
 	return err
 }
 
@@ -1158,30 +1158,15 @@ func (c *Catalog) Merge(ctx context.Context, repository string, destinationBranc
 	}); err != nil {
 		return nil, err
 	}
-	commitID, summary, err := c.Store.Merge(ctx, repositoryID, destination, source, commitParams)
+	commitID, err := c.Store.Merge(ctx, repositoryID, destination, source, commitParams)
 	if errors.Is(err, graveler.ErrConflictFound) {
 		// for compatibility with old Catalog
-		return &MergeResult{
-			Summary: map[DifferenceType]int{
-				DifferenceTypeConflict: summary.Count[graveler.DiffTypeConflict],
-			},
-		}, err
+		return &MergeResult{}, err
 	}
 	if err != nil {
 		return nil, err
 	}
-	count := make(map[DifferenceType]int)
-	for k, v := range summary.Count {
-		kk, err := catalogDiffType(k)
-		if err != nil {
-			return nil, err
-		}
-		count[kk] = v
-	}
-	return &MergeResult{
-		Summary:   count,
-		Reference: commitID.String(),
-	}, nil
+	return &MergeResult{Reference: commitID.String()}, nil
 }
 
 func (c *Catalog) DumpCommits(ctx context.Context, repositoryID string) (string, error) {

pkg/catalog/fake_graveler_test.go

@@ -217,11 +217,11 @@ func (g *FakeGraveler) ResetPrefix(ctx context.Context, repositoryID graveler.Re
 	panic("implement me")
 }
 
-func (g *FakeGraveler) Revert(_ context.Context, _ graveler.RepositoryID, _ graveler.BranchID, _ graveler.Ref, _ int, _ graveler.CommitParams) (graveler.CommitID, graveler.DiffSummary, error) {
+func (g *FakeGraveler) Revert(_ context.Context, _ graveler.RepositoryID, _ graveler.BranchID, _ graveler.Ref, _ int, _ graveler.CommitParams) (graveler.CommitID, error) {
 	panic("implement me")
 }
 
-func (g *FakeGraveler) Merge(ctx context.Context, repositoryID graveler.RepositoryID, destination graveler.BranchID, source graveler.Ref, _ graveler.CommitParams) (graveler.CommitID, graveler.DiffSummary, error) {
+func (g *FakeGraveler) Merge(ctx context.Context, repositoryID graveler.RepositoryID, destination graveler.BranchID, source graveler.Ref, _ graveler.CommitParams) (graveler.CommitID, error) {
 	panic("implement me")
 }
 

pkg/catalog/model.go

@@ -46,7 +46,6 @@ type CommitLog struct {
 }
 
 type MergeResult struct {
-	Summary   map[DifferenceType]int
 	Reference string
 }