mirror of
https://github.com/dolthub/dolt.git
synced 2026-03-15 19:31:03 -05:00
Revert "Change parent order processing in commit iterator so that we always traverse first the parent of a merge commit that supplied the commits merged in that commit."
This reverts commit 2e5e6aa968.
This commit is contained in:
@@ -16,7 +16,6 @@ package doltdb
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"io"
|
||||
|
||||
"github.com/dolthub/go-mysql-server/sql"
|
||||
@@ -114,41 +113,18 @@ func (cmItr *commitItr) Next(ctx context.Context) (hash.Hash, *Commit, error) {
|
||||
}
|
||||
|
||||
parents, err := cmItr.curr.ParentHashes(ctx)
|
||||
|
||||
if err != nil {
|
||||
return hash.Hash{}, nil, err
|
||||
}
|
||||
|
||||
// Add the first parent to the back of the unprocessed queue (i.e. the front of the slice)
|
||||
// so that we only process it AFTER we have processed everything else in front if it in the queue.
|
||||
if len(parents) > 0 {
|
||||
h := parents[0]
|
||||
if !cmItr.added[h] {
|
||||
cmItr.added[h] = true
|
||||
cmItr.unprocessed = append([]hash.Hash{h}, cmItr.unprocessed...)
|
||||
}
|
||||
}
|
||||
|
||||
// For merge commits, we always want to process the second parent BEFORE the first parent, because the
|
||||
// second parent contains the commits that were merged from the source branch into the destination branch.
|
||||
// Processing these first ensures that we attribute data changes to the most specific commit, and not more
|
||||
// generally to a merge commit. To prioritize these second parents, we simply add them to the front of the
|
||||
// unprocessed queue (i.e. the back of the slice).
|
||||
if len(parents) > 1 {
|
||||
h := parents[1]
|
||||
for _, h := range parents {
|
||||
if !cmItr.added[h] {
|
||||
cmItr.added[h] = true
|
||||
cmItr.unprocessed = append(cmItr.unprocessed, h)
|
||||
}
|
||||
}
|
||||
|
||||
if len(parents) > 2 {
|
||||
currentCommitHash, err := cmItr.curr.HashOf()
|
||||
if err != nil {
|
||||
return hash.Hash{}, nil, fmt.Errorf("found commit with more than two parents: %s", err.Error())
|
||||
}
|
||||
return hash.Hash{}, nil, fmt.Errorf("found commit %s with more than two parents", currentCommitHash.String())
|
||||
}
|
||||
|
||||
numUnprocessed := len(cmItr.unprocessed)
|
||||
|
||||
if numUnprocessed == 0 {
|
||||
|
||||
@@ -670,83 +670,6 @@ var DiffSystemTableScriptTests = []queries.ScriptTest{
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
// For more complex commit graphs, we need to traverse the graph in the correct order that
|
||||
// ensures diffs will report the most specific commit that changed a row, and not attribute
|
||||
// it to a merge commit that included the more specific commit. This test creates a commit
|
||||
// graph where merge commits are merged back and forth between branches, resulting in the
|
||||
// graph below. For more details, see: https://github.com/dolthub/dolt/issues/6861
|
||||
//
|
||||
// 0 - repo initialization
|
||||
// ↑
|
||||
// A - new table
|
||||
// ↗ ↑ ↖
|
||||
// M1 ↑ ↑ - Merge test into main
|
||||
// ↑ ↑ ↑ ↑
|
||||
// ↑ ↑ B ↑ - new val 1
|
||||
// ↑ ↖ ↗
|
||||
// ↖ M2 - Merge main into test2
|
||||
// ↖ ↑
|
||||
// M3 - Merge test2 into main
|
||||
Name: "diff shows the most specific commit that changed a row",
|
||||
SetUpScript: []string{
|
||||
"create table xy (x int primary key);",
|
||||
"call dolt_commit('-Am', 'new table');",
|
||||
"call dolt_branch('test');",
|
||||
"call dolt_branch('test2');",
|
||||
"call dolt_checkout('test');",
|
||||
"insert into xy values (0);",
|
||||
"call dolt_commit('-am', 'new val 1');",
|
||||
"set @expectedCommit=hashof('HEAD');",
|
||||
"set @expectedParentCommit=hashof('HEAD~');",
|
||||
"call dolt_checkout('main');",
|
||||
},
|
||||
Assertions: []queries.ScriptTestAssertion{
|
||||
// Merge the data change from test into main and assert that the expected commit from test2
|
||||
// is attributed, and not the merge commit.
|
||||
{
|
||||
// No diffs, because there's no data merged into the table yet
|
||||
Query: "select * from dolt_diff_xy;",
|
||||
Expected: []sql.Row{},
|
||||
},
|
||||
{
|
||||
Query: "call dolt_merge('test', '--no-ff', '-message', 'Merge test into main');",
|
||||
Expected: []sql.Row{{doltCommit, 0, 0}},
|
||||
},
|
||||
{
|
||||
Query: "select to_x, from_x, to_commit = @expectedCommit, from_commit = @expectedParentCommit, diff_type from dolt_diff_xy;",
|
||||
Expected: []sql.Row{{0, nil, true, true, "added"}},
|
||||
},
|
||||
// Merge the merge commit on main into test2 and assert that the dolt_diff_<table> system table
|
||||
// still shows the correct commit attributed for inserting the row in the table.
|
||||
{
|
||||
Query: "call dolt_checkout('test2');",
|
||||
Expected: []sql.Row{{0, "Switched to branch 'test2'"}},
|
||||
},
|
||||
{
|
||||
Query: "call dolt_merge('main', '--no-ff', '-message', 'Merge main into test2');",
|
||||
Expected: []sql.Row{{doltCommit, 0, 0}},
|
||||
},
|
||||
{
|
||||
Query: "select to_x, from_x, to_commit = @expectedCommit, from_commit = @expectedParentCommit, diff_type from dolt_diff_xy;",
|
||||
Expected: []sql.Row{{0, nil, true, true, "added"}},
|
||||
},
|
||||
// Merge the merge commit from branch test2 back onto main and assert that the most
|
||||
// specific commit is still attributed for the data change.
|
||||
{
|
||||
Query: "call dolt_checkout('main');",
|
||||
Expected: []sql.Row{{0, "Switched to branch 'main'"}},
|
||||
},
|
||||
{
|
||||
Query: "call dolt_merge('test2', '--no-ff', '-message', 'Merge test2 into main');",
|
||||
Expected: []sql.Row{{doltCommit, 0, 0}},
|
||||
},
|
||||
{
|
||||
Query: "select to_x, from_x, to_commit = @expectedCommit, from_commit = @expectedParentCommit, diff_type from dolt_diff_xy;",
|
||||
Expected: []sql.Row{{0, nil, true, true, "added"}},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
var Dolt1DiffSystemTableScripts = []queries.ScriptTest{
|
||||
|
||||
Reference in New Issue
Block a user