dolthub / dolthub/dolt

Inconsistent detection of merge conflict, depending on direction of merge.

Open
#7,767 0 comments 0 reactions 1 assignee Claimed by @zachmu View on GitHub
bug merge
Dominant language
Go
Stars
24.4k
Forks
873
Avg merge
1d 5h
Merged PRs (30d)
108

Description

Potentially related to https://github.com/dolthub/dolt/issues/7762

Steps to reproduce:

Add the following test to `SchemaChangeTestsBasicCases` in `dolt_queries_schema_merge.go`

```
{
Name: "ambiguous choice of ancestor column",
AncSetUpScript: []string{
"CREATE table t (pk int primary key, col1 int, col2 int);",
"INSERT into t values (1, 10, 100), (2, 20, 200);",
},
RightSetUpScript: []string{
"alter table t drop column col1;",
"insert into t values (3, 30), (4, 40);",
},
LeftSetUpScript: []string{
"alter table t drop column col2;",
"alter table t rename column col1 to col2;",
"insert into t values (5, 50), (6, 60);",
},
Assertions: []queries.ScriptTestAssertion{
{
Query: "call dolt_merge('right');",
ExpectedErrStr: "Merge conflict detected, @autocommit transaction rolled back. @autocommit must be disabled so that merge conflicts can be resolved using the dolt_conflicts and dolt_schema_conflicts tables before manually committing the transaction. Alternatively, to commit transactions with merge conflicts, set @@dolt_allow_commit_conflicts = 1",
},
},
},
```

In this test, both branches have a `col2` column, but these columns correspond to different columns in the ancestor schema. (In the implementation, we track this by giving each column a tag.) It's ambiguous which column in the ancestor should be used for the merge.

I don't know if we explicitly handle this case, and it's not obvious what the correct behavior should be. I think there's two reasonable arguments here:
- Since the column is named "col2" in both branches, the merge should use the column "col2" in the ancestor.
- This is ambiguous and should result in a merge conflict.

Regardless of the "correct" behavior, we're currently bugged. Dolt will do one or the other of these, depending on the merge direction.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.